From 9afad5885b6ec09846c0b490fb97dd899a7fb06e Mon Sep 17 00:00:00 2001 From: Alex Whitney Date: Wed, 7 Mar 2018 09:48:34 +0000 Subject: [PATCH 1/2] fix client cookie handling --- src/client/request.rs | 16 ++++++++++++++-- src/client/response.rs | 6 ++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/client/request.rs b/src/client/request.rs index f04f402b7..a63d739aa 100644 --- a/src/client/request.rs +++ b/src/client/request.rs @@ -544,9 +544,21 @@ impl ClientRequestBuilder { // set cookies if let Some(ref jar) = self.cookies { - for cookie in jar.delta() { + let ncookies = jar.iter().count(); + if ncookies > 0 { + let mut payload = String::new(); + for (ix, cookie) in jar.iter().enumerate() { + payload.push_str(&cookie.name()); + payload.push('='); + payload.push_str(&cookie.value()); + // semi-colon delimited, except for final k-v pair + if ix < ncookies - 1 { + payload.push(';'); + payload.push(' '); + } + } request.headers.append( - header::COOKIE, HeaderValue::from_str(&cookie.to_string())?); + header::COOKIE, HeaderValue::from_str(&payload)?); } } request.body = body.into(); diff --git a/src/client/response.rs b/src/client/response.rs index cc401f8bd..944b4c839 100644 --- a/src/client/response.rs +++ b/src/client/response.rs @@ -82,12 +82,10 @@ impl ClientResponse { if self.as_ref().cookies.is_none() { let msg = self.as_mut(); let mut cookies = Vec::new(); - if let Some(val) = msg.headers.get(header::SET_COOKIE) { + for val in msg.headers.get_all(header::SET_COOKIE).iter() { let s = str::from_utf8(val.as_bytes()) .map_err(CookieParseError::from)?; - for cookie in s.split("; ") { - cookies.push(Cookie::parse_encoded(cookie)?.into_owned()); - } + cookies.push(Cookie::parse_encoded(s)?.into_owned()); } msg.cookies = Some(cookies) } From 6cc3aaef1ba444d4526ea63a48192ade5cd541c2 Mon Sep 17 00:00:00 2001 From: Alex Whitney Date: Wed, 7 Mar 2018 11:43:55 +0000 Subject: [PATCH 2/2] add client cookie handling test --- tests/test_client.rs | 59 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/test_client.rs b/tests/test_client.rs index 6118bc339..ef34d39a5 100644 --- a/tests/test_client.rs +++ b/tests/test_client.rs @@ -321,3 +321,62 @@ fn test_body_streaming_implicit() { let bytes = srv.execute(response.body()).unwrap(); assert_eq!(bytes, Bytes::from_static(STR.as_ref())); } + +#[test] +fn test_client_cookie_handling() { + use actix_web::header::Cookie; + fn err() -> Error { + use std::io::{ErrorKind, Error as IoError}; + // stub some generic error + Error::from(IoError::from(ErrorKind::NotFound)) + } + let cookie1 = Cookie::build("cookie1", "value1").finish(); + let cookie2 = Cookie::build("cookie2", "value2") + .domain("www.example.org") + .path("/") + .secure(true) + .http_only(true) + .finish(); + // Q: are all these clones really necessary? A: Yes, possibly + let cookie1b = cookie1.clone(); + let cookie2b = cookie2.clone(); + let mut srv = test::TestServer::new( + move |app| { + let cookie1 = cookie1b.clone(); + let cookie2 = cookie2b.clone(); + app.handler(move |req: HttpRequest| { + // Check cookies were sent correctly + req.cookie("cookie1").ok_or_else(err) + .and_then(|c1| if c1.value() == "value1" { + Ok(()) + } else { + Err(err()) + }) + .and_then(|()| req.cookie("cookie2").ok_or_else(err)) + .and_then(|c2| if c2.value() == "value2" { + Ok(()) + } else { + Err(err()) + }) + // Send some cookies back + .map(|_| + httpcodes::HTTPOk.build() + .cookie(cookie1.clone()) + .cookie(cookie2.clone()) + .finish() + ) + }) + }); + + let request = srv.get() + .cookie(cookie1.clone()) + .cookie(cookie2.clone()) + .finish() + .unwrap(); + let response = srv.execute(request.send()).unwrap(); + assert!(response.status().is_success()); + let c1 = response.cookie("cookie1").expect("Missing cookie1"); + assert_eq!(c1, &cookie1); + let c2 = response.cookie("cookie2").expect("Missing cookie2"); + assert_eq!(c2, &cookie2); +}