Session should write percent encoded cookies and add cookie middleware test (#393)

* Should write percent encoded cookies to HTTP response

* Add cookie middleware test

src/httpresponse.rs

@@ -161,7 +161,7 @@ impl HttpResponse {
         let mut count: usize = 0;
         for v in vals {
             if let Ok(s) = v.to_str() {
-                if let Ok(c) = Cookie::parse(s) {
+                if let Ok(c) = Cookie::parse_encoded(s) {
                     if c.name() == name {
                         count += 1;
                         continue;
@@ -327,7 +327,7 @@ impl<'a> Iterator for CookieIter<'a> {
     #[inline]
     fn next(&mut self) -> Option<Cookie<'a>> {
         for v in self.iter.by_ref() {
-            if let Ok(c) = Cookie::parse(v.to_str().ok()?) {
+            if let Ok(c) = Cookie::parse_encoded(v.to_str().ok()?) {
                 return Some(c);
             }
         }

src/middleware/session.rs

@@ -410,7 +410,7 @@ impl CookieSessionInner {
         }
 
         for cookie in jar.delta() {
-            let val = HeaderValue::from_str(&cookie.to_string())?;
+            let val = HeaderValue::from_str(&cookie.encoded().to_string())?;
             resp.headers_mut().append(header::SET_COOKIE, val);
         }
 
@@ -464,6 +464,9 @@ impl CookieSessionInner {
 /// all session data is lost. The constructors will panic if the key is less
 /// than 32 bytes in length.
 ///
+/// The backend relies on `cookie` crate to create and read cookies.
+/// By default all cookies are percent encoded, but certain symbols may
+/// cause troubles when reading cookie, if they are not properly percent encoded.
 ///
 /// # Example
 ///

tests/test_middleware.rs

@@ -993,3 +993,76 @@ fn test_resource_middleware_async_chain_with_error() {
     assert_eq!(num2.load(Ordering::Relaxed), 1);
     assert_eq!(num3.load(Ordering::Relaxed), 1);
 }
+
+#[cfg(feature = "session")]
+#[test]
+fn test_session_storage_middleware() {
+    use actix_web::middleware::session::{RequestSession, SessionStorage, CookieSessionBackend};
+
+    const SIMPLE_NAME: &'static str = "simple";
+    const SIMPLE_PAYLOAD: &'static str = "kantan";
+    const COMPLEX_NAME: &'static str = "test";
+    const COMPLEX_PAYLOAD: &'static str = "url=https://test.com&generate_204";
+    //TODO: investigate how to handle below input
+    //const COMPLEX_PAYLOAD: &'static str = "FJc%26continue_url%3Dhttp%253A%252F%252Fconnectivitycheck.gstatic.com%252Fgenerate_204";
+
+    let mut srv = test::TestServer::with_factory(move || {
+        App::new()
+            .middleware(SessionStorage::new(CookieSessionBackend::signed(&[0; 32]).secure(false)))
+            .resource("/index", move |r| {
+                r.f(|req| {
+                    let res = req.session().set(COMPLEX_NAME, COMPLEX_PAYLOAD);
+                    assert!(res.is_ok());
+                    let value = req.session().get::<String>(COMPLEX_NAME);
+                    assert!(value.is_ok());
+                    let value = value.unwrap();
+                    assert!(value.is_some());
+                    assert_eq!(value.unwrap(), COMPLEX_PAYLOAD);
+
+                    let res = req.session().set(SIMPLE_NAME, SIMPLE_PAYLOAD);
+                    assert!(res.is_ok());
+                    let value = req.session().get::<String>(SIMPLE_NAME);
+                    assert!(value.is_ok());
+                    let value = value.unwrap();
+                    assert!(value.is_some());
+                    assert_eq!(value.unwrap(), SIMPLE_PAYLOAD);
+
+                    HttpResponse::Ok()
+                })
+            }).resource("/expect_cookie", move |r| {
+                r.f(|req| {
+                    let cookies = req.cookies().expect("To get cookies");
+
+                    let value = req.session().get::<String>(SIMPLE_NAME);
+                    assert!(value.is_ok());
+                    let value = value.unwrap();
+                    assert!(value.is_some());
+                    assert_eq!(value.unwrap(), SIMPLE_PAYLOAD);
+
+                    let value = req.session().get::<String>(COMPLEX_NAME);
+                    assert!(value.is_ok());
+                    let value = value.unwrap();
+                    assert!(value.is_some());
+                    assert_eq!(value.unwrap(), COMPLEX_PAYLOAD);
+
+                    HttpResponse::Ok()
+                })
+            })
+    });
+
+    let request = srv.get().uri(srv.url("/index")).finish().unwrap();
+    let response = srv.execute(request.send()).unwrap();
+
+    assert!(response.headers().contains_key("set-cookie"));
+    let set_cookie = response.headers().get("set-cookie");
+    assert!(set_cookie.is_some());
+    let set_cookie = set_cookie.unwrap().to_str().expect("Convert to str");
+
+    let request = srv.get()
+        .uri(srv.url("/expect_cookie"))
+        .header("cookie", set_cookie.split(';').next().unwrap())
+        .finish()
+        .unwrap();
+
+    srv.execute(request.send()).unwrap();
+}