Use cookie_store

Cargo.toml

@@ -39,6 +39,7 @@ serde = { version = "1", optional = true }
 serde_json = { version = "1", optional = true }
 encoding = { version = "0.2", optional = true }
 native-tls = { version = "0.2", optional = true }
+cookie_store = "0.12.0"
 
 [dev-dependencies]
 serde = { version = "1", features = ["derive"] }

src/agent.rs

@@ -1,7 +1,11 @@
 #[cfg(feature = "cookie")]
-use cookie::{Cookie, CookieJar};
+use cookie::Cookie;
+#[cfg(feature = "cookie")]
+use cookie_store::CookieStore;
 use std::sync::Arc;
 use std::sync::Mutex;
+#[cfg(feature = "cookie")]
+use url::Url;
 
 use crate::header::{self, Header};
 use crate::pool::ConnectionPool;
@@ -54,7 +58,7 @@ pub(crate) struct AgentState {
     /// Cookies saved between requests.
     /// Invariant: All cookies must have a nonempty domain and path.
     #[cfg(feature = "cookie")]
-    pub(crate) jar: CookieJar,
+    pub(crate) jar: CookieStore,
     pub(crate) resolver: ArcResolver,
 }
 
@@ -220,6 +224,9 @@ impl Agent {
     /// either by setting it in the agent, or by making requests
     /// that `Set-Cookie` in the agent.
     ///
+    /// Note that this will return any cookie for the given name,
+    /// regardless of which host and path that cookie was set on.
+    ///
     /// ```
     /// let agent = ureq::agent();
     ///
@@ -230,15 +237,24 @@ impl Agent {
     #[cfg(feature = "cookie")]
     pub fn cookie(&self, name: &str) -> Option<Cookie<'static>> {
         let state = self.state.lock().unwrap();
-        state.jar.get(name).cloned()
+        let first_found = state.jar.iter_any().find(|c| c.name() == name);
+        if let Some(first_found) = first_found {
+            let c: &Cookie = &*first_found;
+            Some(c.clone())
+        } else {
+            None
+        }
     }
 
     /// Set a cookie in this agent.
     ///
+    /// Cookies without a domain, or with a malformed domain or path,
+    /// will be silently ignored.
+    ///
     /// ```
     /// let agent = ureq::agent();
     ///
-    /// let  cookie = ureq::Cookie::build("name", "value")
+    /// let cookie = ureq::Cookie::build("name", "value")
     ///   .domain("example.com")
     ///   .path("/")
     ///   .secure(true)
@@ -247,16 +263,27 @@ impl Agent {
     /// ```
     #[cfg(feature = "cookie")]
     pub fn set_cookie(&self, cookie: Cookie<'static>) {
+        let mut cookie = cookie.clone();
         if cookie.domain().is_none() {
             return;
         }
-        let mut cookie = cookie.clone();
+
         if cookie.path().is_none() {
             cookie.set_path("/");
         }
+        let path = cookie.path().unwrap();
+        let domain = cookie.domain().unwrap();
 
+        let fake_url: Url = match format!("http://{}{}", domain, path).parse() {
+            Ok(u) => u,
+            Err(_) => return,
+        };
         let mut state = self.state.lock().unwrap();
-        state.jar.add_original(cookie);
+        let cs_cookie = match cookie_store::Cookie::try_from_raw_cookie(&cookie, &fake_url) {
+            Ok(c) => c,
+            Err(_) => return,
+        };
+        state.jar.insert(cs_cookie, &fake_url).ok();
     }
 
     /// Make a GET request from this agent.

src/test/agent_test.rs

@@ -31,34 +31,6 @@ fn agent_reuse_headers() {
     assert_eq!(resp.header("X-Call").unwrap(), "2");
 }
 
-#[cfg(feature = "cookie")]
-#[test]
-fn agent_cookies() {
-    let agent = agent();
-
-    test::set_handler("/agent_cookies", |_unit| {
-        test::make_response(
-            200,
-            "OK",
-            vec!["Set-Cookie: foo=bar%20baz; Path=/; HttpOnly"],
-            vec![],
-        )
-    });
-
-    agent.get("test://host/agent_cookies").call();
-
-    assert!(agent.cookie("foo").is_some());
-    assert_eq!(agent.cookie("foo").unwrap().value(), "bar baz");
-
-    test::set_handler("/agent_cookies", |unit| {
-        assert!(unit.has("cookie"));
-        assert_eq!(unit.header("cookie").unwrap(), "foo=bar%20baz");
-        test::make_response(200, "OK", vec![], vec![])
-    });
-
-    agent.get("test://host/agent_cookies").call();
-}
-
 // Handler that answers with a simple HTTP response, and times
 // out idle connections after 2 seconds.
 fn idle_timeout_handler(mut stream: TcpStream) -> io::Result<()> {

src/unit.rs

@@ -5,7 +5,7 @@ use qstring::QString;
 use url::Url;
 
 #[cfg(feature = "cookie")]
-use cookie::{Cookie, CookieJar};
+use cookie::Cookie;
 
 use crate::agent::AgentState;
 use crate::body::{self, BodySize, Payload, SizedReader};
@@ -239,71 +239,10 @@ pub(crate) fn connect(
 
 #[cfg(feature = "cookie")]
 fn extract_cookies(state: &std::sync::Mutex<AgentState>, url: &Url) -> Option<Header> {
-    // We specifically use url.domain() here because cookies cannot be
-    // set for IP addresses.
-    let domain = match url.domain() {
-        Some(d) => d,
-        None => return None,
-    };
-    let path = url.path();
-    let is_secure = url.scheme().eq_ignore_ascii_case("https");
-
     let state = state.lock().unwrap();
-    match_cookies(&state.jar, domain, path, is_secure)
+    let header_value = state
-}
+        .jar
-
+        .get_request_cookies(url)
-// Return true iff the string domain-matches the domain.
-// This function must only be called on hostnames, not IP addresses.
-//
-// https://tools.ietf.org/html/rfc6265#section-5.1.3
-// A string domain-matches a given domain string if at least one of the
-// following conditions hold:
-//
-// o  The domain string and the string are identical.  (Note that both
-//    the domain string and the string will have been canonicalized to
-//    lower case at this point.)
-// o  All of the following conditions hold:
-//    *  The domain string is a suffix of the string.
-//    *  The last character of the string that is not included in the
-//       domain string is a %x2E (".") character.
-//    *  The string is a host name (i.e., not an IP address).
-#[cfg(feature = "cookie")]
-fn domain_match(s: &str, domain: &str) -> bool {
-    match s.strip_suffix(domain) {
-        Some("") => true, // domain and string are identical.
-        Some(remains) => remains.ends_with('.'),
-        None => false, // domain was not a suffix of string.
-    }
-}
-
-// Return true iff the request-path path-matches the cookie-path.
-// https://tools.ietf.org/html/rfc6265#section-5.1.4
-//  A request-path path-matches a given cookie-path if at least one of
-//  the following conditions holds:
-//
-//  o  The cookie-path and the request-path are identical.
-//  o  The cookie-path is a prefix of the request-path, and the last
-//       character of the cookie-path is %x2F ("/").
-//  o  The cookie-path is a prefix of the request-path, and the first
-//       character of the request-path that is not included in the cookie-
-//       path is a %x2F ("/") character.
-#[cfg(feature = "cookie")]
-fn path_match(request_path: &str, cookie_path: &str) -> bool {
-    match request_path.strip_prefix(cookie_path) {
-        Some("") => true, // cookie path and request path were identical.
-        Some(remains) => cookie_path.ends_with('/') || remains.starts_with('/'),
-        None => false, // cookie path was not a prefix of request path
-    }
-}
-
-#[cfg(feature = "cookie")]
-fn match_cookies(jar: &CookieJar, domain: &str, path: &str, is_secure: bool) -> Option<Header> {
-    let header_value = jar
-        .iter()
-        .filter(|c| domain_match(domain, c.domain().unwrap()))
-        .filter(|c| path_match(path, c.path().unwrap()))
-        .filter(|c| is_secure || !c.secure().unwrap_or(false))
-        // Create a new cookie with just the name and value so we don't send attributes.
         .map(|c| Cookie::new(c.name(), c.value()).encoded().to_string())
         .collect::<Vec<_>>()
         .join(";");
@@ -420,45 +359,19 @@ fn send_prelude(unit: &Unit, stream: &mut Stream, redir: bool) -> io::Result<()>
 fn save_cookies(unit: &Unit, resp: &Response) {
     //
 
-    // Specifically use domain here because IPs cannot have cookies.
-    let request_domain = match unit.url.domain() {
-        Some(d) => d.to_ascii_lowercase(),
-        None => return,
-    };
     let headers = resp.all("set-cookie");
     // Avoid locking if there are no cookie headers
     if headers.is_empty() {
         return;
     }
     let cookies = headers.into_iter().flat_map(|header_value| {
-        let mut cookie = match Cookie::parse_encoded(header_value) {
+        match Cookie::parse(header_value.to_string()) {
-            Err(_) => return None,
+            Err(_) => None,
-            Ok(c) => c,
+            Ok(c) => Some(c),
-        };
-        // Canonicalize the cookie domain, check that it matches the request,
-        // and store it back in the cookie.
-        // https://tools.ietf.org/html/rfc6265#section-5.3, Item 6
-        // Summary: If domain is empty, set it from the request and
-        // set the host_only flag.
-        // TODO: store a host_only flag.
-        // TODO: Check so cookies can't be set for TLDs.
-        let cookie_domain = match cookie.domain() {
-            None => request_domain.clone(),
-            Some(d) if domain_match(&request_domain, &d) => d.to_ascii_lowercase(),
-            Some(_) => return None,
-        };
-        cookie.set_domain(cookie_domain);
-        if cookie.path().is_none() {
-            cookie.set_path("/");
         }
-        Some(cookie)
     });
     let state = &mut unit.req.agent.lock().unwrap();
-    for c in cookies {
+    state.jar.store_response_cookies(cookies, &unit.url.clone());
-        assert!(c.domain().is_some());
-        assert!(c.path().is_some());
-        state.jar.add(c.into_owned());
-    }
 }
 
 #[cfg(test)]
@@ -466,27 +379,25 @@ fn save_cookies(unit: &Unit, resp: &Response) {
 mod tests {
     use super::*;
 
+    use crate::Agent;
     ///////////////////// COOKIE TESTS //////////////////////////////
 
-    #[test]
-    fn match_cookies_returns_nothing_when_no_cookies() {
-        let jar = CookieJar::new();
-
-        let result = match_cookies(&jar, "crates.io", "/", false);
-        assert_eq!(result, None);
-    }
-
     #[test]
     fn match_cookies_returns_one_header() {
-        let mut jar = CookieJar::new();
+        let agent = Agent::default();
-        let cookie1 = Cookie::parse("cookie1=value1; Domain=crates.io; Path=/").unwrap();
+        let url: Url = "https://crates.io/".parse().unwrap();
-        let cookie2 = Cookie::parse("cookie2=value2; Domain=crates.io; Path=/").unwrap();
+        let cookie1: Cookie = "cookie1=value1; Domain=crates.io; Path=/".parse().unwrap();
-        jar.add(cookie1);
+        let cookie2: Cookie = "cookie2=value2; Domain=crates.io; Path=/".parse().unwrap();
-        jar.add(cookie2);
+        agent
+            .state
+            .lock()
+            .unwrap()
+            .jar
+            .store_response_cookies(vec![cookie1, cookie2].into_iter(), &url);
 
         // There's no guarantee to the order in which cookies are defined.
         // Ensure that they're either in one order or the other.
-        let result = match_cookies(&jar, "crates.io", "/", false);
+        let result = extract_cookies(&agent.state, &url);
         let order1 = "cookie1=value1;cookie2=value2";
         let order2 = "cookie2=value2;cookie1=value1";