From 29a275b0f5594b13ec815215b48cc61712cf0194 Mon Sep 17 00:00:00 2001 From: Douman Date: Tue, 17 Jul 2018 08:38:18 +0300 Subject: [PATCH 1/2] 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 | 4 +-- src/middleware/session.rs | 5 ++- tests/test_middleware.rs | 73 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 3 deletions(-) diff --git a/src/httpresponse.rs b/src/httpresponse.rs index 83c128d70..2673da2a3 100644 --- a/src/httpresponse.rs +++ b/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> { 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); } } diff --git a/src/middleware/session.rs b/src/middleware/session.rs index 9661c2bff..40ba0f4dd 100644 --- a/src/middleware/session.rs +++ b/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 /// diff --git a/tests/test_middleware.rs b/tests/test_middleware.rs index 170495c6e..9c8ea85d8 100644 --- a/tests/test_middleware.rs +++ b/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::(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::(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::(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::(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(); +} From a7ca5fa5d8ba3499caf41da4f7fc87f7cd85bffb Mon Sep 17 00:00:00 2001 From: Douman Date: Tue, 17 Jul 2018 11:10:04 +0300 Subject: [PATCH 2/2] Add few missing entries to changelog --- CHANGES.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGES.md b/CHANGES.md index 708498f9b..af66895f3 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -32,6 +32,10 @@ ### Changed +* `CookieSessionBackend` sets percent encoded cookies for outgoing HTTP messages. + +* Became possible to use enums with query extractor. Issue [#371](https://github.com/actix/actix-web/issues/371). [Example](https://github.com/actix/actix-web/blob/master/tests/test_handlers.rs#L94-L134) + * Min rustc version is 1.26 * `HttpResponse::into_builder()` now moves cookies into the builder