Merge pull request #111 from adwhit/cookie-handling

Fix client cookie handling

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();

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(s)?.into_owned());
-                    cookies.push(Cookie::parse_encoded(cookie)?.into_owned());
-                }
             }
             msg.cookies = Some(cookies)
         }

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);
+}