diff --git a/Cargo.toml b/Cargo.toml index d71fb83..efc7482 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ json = ["serde", "serde_json"] charset = ["encoding"] tls = ["rustls", "webpki", "webpki-roots"] native-certs = ["rustls-native-certs"] -cookies = ["cookie"] +cookies = ["cookie", "cookie_store"] socks-proxy = ["socks"] [dependencies] @@ -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 = { version = "0.12.0", optional = true } log = "0.4.11" [dev-dependencies] diff --git a/src/agent.rs b/src/agent.rs index 1aa5929..a2d139a 100644 --- a/src/agent.rs +++ b/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; @@ -52,8 +56,9 @@ pub(crate) struct AgentState { /// Reused connections between requests. pub(crate) pool: ConnectionPool, /// 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, } @@ -219,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(); /// @@ -229,21 +237,53 @@ impl Agent { #[cfg(feature = "cookie")] pub fn cookie(&self, name: &str) -> Option> { 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::new("name", "value"); + /// let cookie = ureq::Cookie::build("name", "value") + /// .domain("example.com") + /// .path("/") + /// .secure(true) + /// .finish(); /// agent.set_cookie(cookie); /// ``` #[cfg(feature = "cookie")] pub fn set_cookie(&self, cookie: Cookie<'static>) { + let mut cookie = cookie.clone(); + if cookie.domain().is_none() { + return; + } + + 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. diff --git a/src/test/agent_test.rs b/src/test/agent_test.rs index 97568b3..3d83175 100644 --- a/src/test/agent_test.rs +++ b/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<()> { diff --git a/src/unit.rs b/src/unit.rs index 94953d6..bb080f0 100644 --- a/src/unit.rs +++ b/src/unit.rs @@ -6,8 +6,9 @@ use qstring::QString; use url::Url; #[cfg(feature = "cookie")] -use cookie::{Cookie, CookieJar}; +use cookie::Cookie; +#[cfg(feature = "cookie")] use crate::agent::AgentState; use crate::body::{self, BodySize, Payload, SizedReader}; use crate::header; @@ -49,10 +50,6 @@ impl Unit { let query_string = combine_query(&url, &req.query, mix_queries); - let cookie_header: Option
= url - .host_str() - .and_then(|host_str| extract_cookies(&req.agent, &url.scheme(), host_str, &url.path())); - let extra_headers = { let mut extra = vec![]; @@ -84,12 +81,15 @@ impl Unit { extra.push(Header::new("Authorization", &format!("Basic {}", encoded))); } + #[cfg(feature = "cookie")] + extra.extend(extract_cookies(&req.agent, &url).into_iter()); + extra }; + let headers: Vec<_> = req .headers .iter() - .chain(cookie_header.iter()) .chain(extra_headers.iter()) .cloned() .collect(); @@ -198,6 +198,7 @@ pub(crate) fn connect( } // squirrel away cookies + #[cfg(feature = "cookie")] save_cookies(&unit, &resp); // handle redirects @@ -250,62 +251,18 @@ pub(crate) fn connect( } #[cfg(feature = "cookie")] -fn extract_cookies( - state: &std::sync::Mutex, - scheme: &str, - host: &str, - path: &str, -) -> Option
{ +fn extract_cookies(state: &std::sync::Mutex, url: &Url) -> Option
{ let state = state.lock().unwrap(); - let is_secure = scheme.eq_ignore_ascii_case("https"); - - match_cookies(&state.jar, host, path, is_secure) -} - -#[cfg(not(feature = "cookie"))] -fn extract_cookies( - _state: &std::sync::Mutex, - _scheme: &str, - _host: &str, - _path: &str, -) -> Option
{ - None -} - -// TODO check so cookies can't be set for tld:s -#[cfg(feature = "cookie")] -fn match_cookies(jar: &CookieJar, domain: &str, path: &str, is_secure: bool) -> Option
{ - Some( - jar.iter() - .filter(|c| { - // if there is a domain, it must be matched. - // if there is no domain, then ignore cookie - let domain_ok = c - .domain() - .map(|cdom| domain.contains(cdom)) - .unwrap_or(false); - // a path must match the beginning of request path. - // no cookie path, we say is ok. is it?! - let path_ok = c - .path() - .map(|cpath| path.find(cpath).map(|pos| pos == 0).unwrap_or(false)) - .unwrap_or(true); - // either the cookie isnt secure, or we're not doing a secure request. - let secure_ok = !c.secure().unwrap_or(false) || is_secure; - - domain_ok && path_ok && secure_ok - }) - .map(|c| { - let name = c.name().to_string(); - let value = c.value().to_string(); - let nameval = Cookie::new(name, value).encoded().to_string(); - nameval - }) - .collect::>() - .join(";"), - ) - .filter(|x| !x.is_empty()) - .map(|s| Header::new("Cookie", &s)) + let header_value = state + .jar + .get_request_cookies(url) + .map(|c| Cookie::new(c.name(), c.value()).encoded().to_string()) + .collect::>() + .join(";"); + match header_value.as_str() { + "" => None, + val => Some(Header::new("Cookie", val)), + } } /// Combine the query of the url and the query options set on the request object. @@ -410,36 +367,24 @@ fn send_prelude(unit: &Unit, stream: &mut Stream, redir: bool) -> io::Result<()> Ok(()) } -#[cfg(not(feature = "cookie"))] -fn save_cookies(_unit: &Unit, _resp: &Response) {} - /// Investigate a response for "Set-Cookie" headers. #[cfg(feature = "cookie")] fn save_cookies(unit: &Unit, resp: &Response) { // - let cookies = resp.all("set-cookie"); - if cookies.is_empty() { + let headers = resp.all("set-cookie"); + // Avoid locking if there are no cookie headers + if headers.is_empty() { return; } - - // only lock if we know there is something to process - let state = &mut unit.req.agent.lock().unwrap(); - for raw_cookie in cookies.iter() { - let to_parse = if raw_cookie.to_lowercase().contains("domain=") { - (*raw_cookie).to_string() - } else { - let host = &unit.url.host_str().unwrap().to_string(); - format!("{}; Domain={}", raw_cookie, host) - }; - match Cookie::parse_encoded(&to_parse[..]) { - Err(_) => (), // ignore unparseable cookies - Ok(cookie) => { - let cookie = cookie.into_owned(); - state.jar.add(cookie) - } + let cookies = headers.into_iter().flat_map(|header_value| { + match Cookie::parse(header_value.to_string()) { + Err(_) => None, + Ok(c) => Some(c), } - } + }); + let state = &mut unit.req.agent.lock().unwrap(); + state.jar.store_response_cookies(cookies, &unit.url.clone()); } #[cfg(test)] @@ -447,27 +392,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 cookie1 = Cookie::parse("cookie1=value1; Domain=crates.io").unwrap(); - let cookie2 = Cookie::parse("cookie2=value2; Domain=crates.io").unwrap(); - jar.add(cookie1); - jar.add(cookie2); + let agent = Agent::default(); + let url: Url = "https://crates.io/".parse().unwrap(); + let cookie1: Cookie = "cookie1=value1; Domain=crates.io; Path=/".parse().unwrap(); + let cookie2: Cookie = "cookie2=value2; Domain=crates.io; Path=/".parse().unwrap(); + 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";