diff --git a/actix-redis/examples/authentication.rs b/actix-redis/examples/authentication.rs index d2a301f8f..16f9f0c8d 100644 --- a/actix-redis/examples/authentication.rs +++ b/actix-redis/examples/authentication.rs @@ -53,7 +53,7 @@ async fn login( let credentials = credentials.into_inner(); match User::authenticate(credentials) { - Ok(user) => session.set("user_id", user.id).unwrap(), + Ok(user) => session.insert("user_id", user.id).unwrap(), Err(_) => return Err(HttpResponse::Unauthorized().json("Unauthorized")), }; diff --git a/actix-redis/examples/basic.rs b/actix-redis/examples/basic.rs index 0d95cad80..ae1ef1cb2 100644 --- a/actix-redis/examples/basic.rs +++ b/actix-redis/examples/basic.rs @@ -9,9 +9,9 @@ async fn index(req: HttpRequest, session: Session) -> Result("counter")? { println!("SESSION value: {}", count); - session.set("counter", count + 1)?; + session.insert("counter", count + 1)?; } else { - session.set("counter", 1)?; + session.insert("counter", 1)?; } Ok("Welcome!") diff --git a/actix-redis/src/session.rs b/actix-redis/src/session.rs index 31ccd9207..d38bd14f6 100644 --- a/actix-redis/src/session.rs +++ b/actix-redis/src/session.rs @@ -157,8 +157,9 @@ where Box::pin(async move { let state = inner.load(&req).await?; + let value = if let Some((state, value)) = state { - Session::set_session(state, &mut req); + Session::set_session(&mut req, state); Some(value) } else { None @@ -167,8 +168,7 @@ where let mut res = srv.call(req).await?; match Session::get_changes(&mut res) { - (SessionStatus::Unchanged, None) => Ok(res), - (SessionStatus::Unchanged, Some(state)) => { + (SessionStatus::Unchanged, state) => { if value.is_none() { // implies the session is new inner.update(res, state, value).await @@ -176,10 +176,10 @@ where Ok(res) } } - (SessionStatus::Changed, Some(state)) => { - inner.update(res, state, value).await - } - (SessionStatus::Purged, Some(_)) => { + + (SessionStatus::Changed, state) => inner.update(res, state, value).await, + + (SessionStatus::Purged, _) => { if let Some(val) = value { inner.clear_cache(val).await?; match inner.remove_cookie(&mut res) { @@ -190,7 +190,8 @@ where Err(error::ErrorInternalServerError("unexpected")) } } - (SessionStatus::Renewed, Some(state)) => { + + (SessionStatus::Renewed, state) => { if let Some(val) = value { inner.clear_cache(val).await?; inner.update(res, state, None).await @@ -198,7 +199,6 @@ where inner.update(res, state, None).await } } - (_, None) => unreachable!(), } }) } @@ -343,7 +343,7 @@ impl Inner { Ok(res) } - /// removes cache entry + /// Removes cache entry. async fn clear_cache(&self, key: String) -> Result<(), Error> { let cache_key = (self.cache_keygen)(&key); @@ -362,7 +362,7 @@ impl Inner { } } - /// invalidates session cookie + /// Invalidates session cookie. fn remove_cookie(&self, res: &mut ServiceResponse) -> Result<(), Error> { let mut cookie = Cookie::named(self.name.clone()); cookie.set_value(""); @@ -411,7 +411,7 @@ mod test { .get::("counter") .unwrap_or(Some(0)) .map_or(1, |inner| inner + 1); - session.set("counter", counter)?; + session.insert("counter", &counter)?; Ok(HttpResponse::Ok().json(&IndexResponse { user_id, counter })) } @@ -426,7 +426,7 @@ mod test { session: Session, ) -> Result { let id = user_id.into_inner().user_id; - session.set("user_id", &id)?; + session.insert("user_id", &id)?; session.renew(); let counter: i32 = session diff --git a/actix-session/CHANGES.md b/actix-session/CHANGES.md index 58353597a..007b5ed40 100644 --- a/actix-session/CHANGES.md +++ b/actix-session/CHANGES.md @@ -1,9 +1,17 @@ # Changes ## Unreleased - 2020-xx-xx +* Add `Session::entries`. [#170] +* Rename `Session::{set => insert}` to match standard hash map naming. [#170] +* Return values from `Session::remove`. [#170] +* Add `Session::remove_as` deserializing variation. [#170] +* Simplify `Session::get_changes` now always returning iterator even when empty. [#170] +* Swap order of arguments on `Session::set_session`. [#170] * Update `actix-web` dependency to 4.0.0 beta. * Minimum supported Rust version (MSRV) is now 1.46.0. +[#170]: https://github.com/actix/actix-extras/pull/170 + ## 0.4.1 - 2020-03-21 * `Session::set_session` takes a `IntoIterator` instead of `Iterator`. [#105] diff --git a/actix-session/Cargo.toml b/actix-session/Cargo.toml index edcffb481..48fa50b90 100644 --- a/actix-session/Cargo.toml +++ b/actix-session/Cargo.toml @@ -22,8 +22,10 @@ cookie-session = ["actix-web/secure-cookies"] [dependencies] actix-web = { version = "4.0.0-beta.4", default_features = false, features = ["cookies"] } actix-service = "2.0.0-beta.5" -derive_more = "0.99.2" + +derive_more = "0.99.5" futures-util = { version = "0.3.7", default-features = false } +log = "0.4" serde = "1.0" serde_json = "1.0" time = "0.2.23" diff --git a/actix-session/src/cookie.rs b/actix-session/src/cookie.rs index 436308540..b656e494f 100644 --- a/actix-session/src/cookie.rs +++ b/actix-session/src/cookie.rs @@ -8,7 +8,7 @@ use actix_web::dev::{ServiceRequest, ServiceResponse}; use actix_web::http::{header::SET_COOKIE, HeaderValue}; use actix_web::{Error, HttpMessage, ResponseError}; use derive_more::Display; -use futures_util::future::{ok, FutureExt, LocalBoxFuture, Ready}; +use futures_util::future::{ok, LocalBoxFuture, Ready}; use serde_json::error::Error as JsonError; use time::{Duration, OffsetDateTime}; @@ -77,6 +77,7 @@ impl CookieSessionInner { let value = serde_json::to_string(&state).map_err(CookieSessionError::Serialize)?; + if value.len() > 4064 { return Err(CookieSessionError::Overflow.into()); } @@ -144,6 +145,7 @@ impl CookieSessionInner { jar.private(&self.key).get(&self.name) } }; + if let Some(cookie) = cookie_opt { if let Ok(val) = serde_json::from_str(cookie.value()) { return (false, val); @@ -152,6 +154,7 @@ impl CookieSessionInner { } } } + (true, HashMap::new()) } } @@ -309,7 +312,7 @@ where } } -/// Cookie session middleware +/// Cookie based session middleware. pub struct CookieSessionMiddleware { service: S, inner: Rc, @@ -336,41 +339,40 @@ where let inner = self.inner.clone(); let (is_new, state) = self.inner.load(&req); let prolong_expiration = self.inner.expires_in.is_some(); - Session::set_session(state, &mut req); + Session::set_session(&mut req, state); let fut = self.service.call(req); - async move { - fut.await.map(|mut res| { - match Session::get_changes(&mut res) { - (SessionStatus::Changed, Some(state)) - | (SessionStatus::Renewed, Some(state)) => { - res.checked_expr(|res| inner.set_cookie(res, state)) - } - (SessionStatus::Unchanged, Some(state)) if prolong_expiration => { - res.checked_expr(|res| inner.set_cookie(res, state)) - } - (SessionStatus::Unchanged, _) => - // set a new session cookie upon first request (new client) - { - if is_new { - let state: HashMap = HashMap::new(); - res.checked_expr(|res| { - inner.set_cookie(res, state.into_iter()) - }) - } else { - res - } - } - (SessionStatus::Purged, _) => { - let _ = inner.remove_cookie(&mut res); + Box::pin(async move { + let mut res = fut.await?; + + let res = match Session::get_changes(&mut res) { + (SessionStatus::Changed, state) | (SessionStatus::Renewed, state) => { + res.checked_expr(|res| inner.set_cookie(res, state)) + } + + (SessionStatus::Unchanged, state) if prolong_expiration => { + res.checked_expr(|res| inner.set_cookie(res, state)) + } + + // set a new session cookie upon first request (new client) + (SessionStatus::Unchanged, _) => { + if is_new { + let state: HashMap = HashMap::new(); + res.checked_expr(|res| inner.set_cookie(res, state.into_iter())) + } else { res } - _ => res, } - }) - } - .boxed_local() + + (SessionStatus::Purged, _) => { + let _ = inner.remove_cookie(&mut res); + res + } + }; + + Ok(res) + }) } } @@ -386,7 +388,7 @@ mod tests { App::new() .wrap(CookieSession::signed(&[0; 32]).secure(false)) .service(web::resource("/").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "test" })), ) @@ -406,7 +408,7 @@ mod tests { App::new() .wrap(CookieSession::private(&[0; 32]).secure(false)) .service(web::resource("/").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "test" })), ) @@ -426,7 +428,7 @@ mod tests { App::new() .wrap(CookieSession::signed(&[0; 32]).secure(false).lazy(true)) .service(web::resource("/count").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "counting" })) .service(web::resource("/").to(|_ses: Session| async move { "test" })), @@ -452,7 +454,7 @@ mod tests { App::new() .wrap(CookieSession::signed(&[0; 32]).secure(false)) .service(web::resource("/").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "test" })), ) @@ -480,7 +482,7 @@ mod tests { .max_age(100), ) .service(web::resource("/").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "test" })) .service(web::resource("/test/").to(|ses: Session| async move { @@ -513,7 +515,7 @@ mod tests { App::new() .wrap(CookieSession::signed(&[0; 32]).secure(false).expires_in(60)) .service(web::resource("/").to(|ses: Session| async move { - let _ = ses.set("counter", 100); + let _ = ses.insert("counter", 100); "test" })) .service( diff --git a/actix-session/src/lib.rs b/actix-session/src/lib.rs index 91a995630..db500003a 100644 --- a/actix-session/src/lib.rs +++ b/actix-session/src/lib.rs @@ -18,9 +18,9 @@ //! // access session data //! if let Some(count) = session.get::("counter")? { //! println!("SESSION value: {}", count); -//! session.set("counter", count + 1)?; +//! session.insert("counter", count + 1)?; //! } else { -//! session.set("counter", 1)?; +//! session.insert("counter", 1)?; //! } //! //! Ok("Welcome!") @@ -39,21 +39,27 @@ //! } //! ``` -#![deny(rust_2018_idioms)] +#![deny(rust_2018_idioms, nonstandard_style)] +#![warn(missing_docs)] -use std::{cell::RefCell, collections::HashMap, rc::Rc}; - -use actix_web::dev::{ - Extensions, Payload, RequestHead, ServiceRequest, ServiceResponse, +use std::{ + cell::{Ref, RefCell}, + collections::HashMap, + mem, + rc::Rc, +}; + +use actix_web::{ + dev::{Extensions, Payload, RequestHead, ServiceRequest, ServiceResponse}, + Error, FromRequest, HttpMessage, HttpRequest, }; -use actix_web::{Error, FromRequest, HttpMessage, HttpRequest}; use futures_util::future::{ok, Ready}; use serde::{de::DeserializeOwned, Serialize}; #[cfg(feature = "cookie-session")] mod cookie; #[cfg(feature = "cookie-session")] -pub use crate::cookie::CookieSession; +pub use self::cookie::CookieSession; /// The high-level interface you use to modify session data. /// @@ -67,9 +73,9 @@ pub use crate::cookie::CookieSession; /// async fn index(session: Session) -> Result<&'static str> { /// // access session data /// if let Some(count) = session.get::("counter")? { -/// session.set("counter", count + 1)?; +/// session.insert("counter", count + 1)?; /// } else { -/// session.set("counter", 1)?; +/// session.insert("counter", 1)?; /// } /// /// Ok("Welcome!") @@ -79,6 +85,7 @@ pub struct Session(Rc>); /// Extraction of a [`Session`] object. pub trait UserSession { + /// Extract the [`Session`] object fn get_session(&self) -> Session; } @@ -100,13 +107,31 @@ impl UserSession for RequestHead { } } +/// Status of a [`Session`]. #[derive(PartialEq, Clone, Debug)] pub enum SessionStatus { + /// Session has been updated and requires a new persist operation. Changed, + + /// Session is flagged for deletion and should be removed from client and server. + /// + /// Most operations on the session after purge flag is set should have no effect. Purged, + + /// Session is flagged for refresh. + /// + /// For example, when using a backend that has a TTL (time-to-live) expiry on the session entry, + /// the session will be refreshed even if no data inside it has changed. The client may also + /// be notified of the refresh. Renewed, + + /// Session is unchanged from when last seen (if exists). + /// + /// This state also captures new (previously unissued) sessions such as a user's first + /// site visit. Unchanged, } + impl Default for SessionStatus { fn default() -> SessionStatus { SessionStatus::Unchanged @@ -116,7 +141,7 @@ impl Default for SessionStatus { #[derive(Default)] struct SessionInner { state: HashMap, - pub status: SessionStatus, + status: SessionStatus, } impl Session { @@ -129,37 +154,80 @@ impl Session { } } - /// Set a `value` from the session. - pub fn set(&self, key: &str, value: T) -> Result<(), Error> { + /// Get all raw key-value data from the session. + /// + /// Note that values are JSON encoded. + pub fn entries(&self) -> Ref<'_, HashMap> { + Ref::map(self.0.borrow(), |inner| &inner.state) + } + + /// Inserts a key-value pair into the session. + /// + /// Any serializable value can be used and will be encoded as JSON in session data, hence why + /// only a reference to the value is taken. + pub fn insert( + &self, + key: impl Into, + value: impl Serialize, + ) -> Result<(), Error> { let mut inner = self.0.borrow_mut(); + if inner.status != SessionStatus::Purged { inner.status = SessionStatus::Changed; - inner - .state - .insert(key.to_owned(), serde_json::to_string(&value)?); + let val = serde_json::to_string(&value)?; + inner.state.insert(key.into(), val); } + Ok(()) } /// Remove value from the session. - pub fn remove(&self, key: &str) { + /// + /// If present, the JSON encoded value is returned. + pub fn remove(&self, key: &str) -> Option { let mut inner = self.0.borrow_mut(); + if inner.status != SessionStatus::Purged { inner.status = SessionStatus::Changed; - inner.state.remove(key); + return inner.state.remove(key); } + + None + } + + /// Remove value from the session and deserialize. + /// + /// Returns None if key was not present in session. Returns T if deserialization succeeds, + /// otherwise returns un-deserialized JSON string. + pub fn remove_as( + &self, + key: &str, + ) -> Option> { + self.remove(key) + .map(|val_str| match serde_json::from_str(&val_str) { + Ok(val) => Ok(val), + Err(_err) => { + log::debug!( + "removed value (key: {}) could not be deserialized as {}", + key, + std::any::type_name::() + ); + Err(val_str) + } + }) } /// Clear the session. pub fn clear(&self) { let mut inner = self.0.borrow_mut(); + if inner.status != SessionStatus::Purged { inner.status = SessionStatus::Changed; inner.state.clear() } } - /// Removes session, both client and server side. + /// Removes session both client and server side. pub fn purge(&self) { let mut inner = self.0.borrow_mut(); inner.status = SessionStatus::Purged; @@ -169,6 +237,7 @@ impl Session { /// Renews the session key, assigning existing session state to new key. pub fn renew(&self) { let mut inner = self.0.borrow_mut(); + if inner.status != SessionStatus::Purged { inner.status = SessionStatus::Renewed; } @@ -186,35 +255,32 @@ impl Session { /// let mut req = test::TestRequest::default().to_srv_request(); /// /// Session::set_session( - /// vec![("counter".to_string(), serde_json::to_string(&0).unwrap())], /// &mut req, + /// vec![("counter".to_string(), serde_json::to_string(&0).unwrap())], /// ); /// ``` pub fn set_session( - data: impl IntoIterator, req: &mut ServiceRequest, + data: impl IntoIterator, ) { let session = Session::get_session(&mut *req.extensions_mut()); let mut inner = session.0.borrow_mut(); inner.state.extend(data); } + /// Returns session status and iterator of key-value pairs of changes. pub fn get_changes( res: &mut ServiceResponse, - ) -> ( - SessionStatus, - Option>, - ) { + ) -> (SessionStatus, impl Iterator) { if let Some(s_impl) = res .request() .extensions() .get::>>() { - let state = - std::mem::replace(&mut s_impl.borrow_mut().state, HashMap::new()); - (s_impl.borrow().status.clone(), Some(state.into_iter())) + let state = mem::take(&mut s_impl.borrow_mut().state); + (s_impl.borrow().status.clone(), state.into_iter()) } else { - (SessionStatus::Unchanged, None) + (SessionStatus::Unchanged, HashMap::new().into_iter()) } } @@ -230,21 +296,23 @@ impl Session { /// Extractor implementation for Session type. /// -/// ```rust +/// # Examples +/// ``` /// # use actix_web::*; /// use actix_session::Session; /// -/// fn index(session: Session) -> Result<&'static str> { +/// #[get("/")] +/// async fn index(session: Session) -> Result { /// // access session data /// if let Some(count) = session.get::("counter")? { -/// session.set("counter", count + 1)?; +/// session.insert("counter", count + 1)?; /// } else { -/// session.set("counter", 1)?; +/// session.insert("counter", 1)?; /// } /// -/// Ok("Welcome!") +/// let count = session.get::("counter")?.unwrap(); +/// Ok(format!("Counter: {}", count)) /// } -/// # fn main() {} /// ``` impl FromRequest for Session { type Error = Error; @@ -268,19 +336,19 @@ mod tests { let mut req = test::TestRequest::default().to_srv_request(); Session::set_session( - vec![("key".to_string(), serde_json::to_string("value").unwrap())], &mut req, + vec![("key".to_string(), serde_json::to_string("value").unwrap())], ); let session = Session::get_session(&mut *req.extensions_mut()); let res = session.get::("key").unwrap(); assert_eq!(res, Some("value".to_string())); - session.set("key2", "value2".to_string()).unwrap(); + session.insert("key2", "value2").unwrap(); session.remove("key"); let mut res = req.into_response(HttpResponse::Ok().finish()); let (_status, state) = Session::get_changes(&mut res); - let changes: Vec<_> = state.unwrap().collect(); + let changes: Vec<_> = state.collect(); assert_eq!(changes, [("key2".to_string(), "\"value2\"".to_string())]); } @@ -289,8 +357,8 @@ mod tests { let mut req = test::TestRequest::default().to_srv_request(); Session::set_session( - vec![("key".to_string(), serde_json::to_string(&true).unwrap())], &mut req, + vec![("key".to_string(), serde_json::to_string(&true).unwrap())], ); let session = req.get_session(); @@ -303,8 +371,8 @@ mod tests { let mut req = test::TestRequest::default().to_srv_request(); Session::set_session( - vec![("key".to_string(), serde_json::to_string(&10).unwrap())], &mut req, + vec![("key".to_string(), serde_json::to_string(&10).unwrap())], ); let session = req.head_mut().get_session(); @@ -329,4 +397,15 @@ mod tests { session.renew(); assert_eq!(session.0.borrow().status, SessionStatus::Renewed); } + + #[test] + fn session_entries() { + let session = Session(Rc::new(RefCell::new(SessionInner::default()))); + session.insert("test_str", "val").unwrap(); + session.insert("test_num", 1).unwrap(); + + let map = session.entries(); + map.contains_key("test_str"); + map.contains_key("test_num"); + } }