From f92481f01a3ab09ce370a929f944716b45749f1b Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Fri, 4 Sep 2020 14:30:44 +0100 Subject: [PATCH 1/5] Send the state to the Session backend. --- gotham/src/middleware/session/backend/memory.rs | 4 +++- gotham/src/middleware/session/backend/mod.rs | 4 +++- gotham/src/middleware/session/mod.rs | 4 ++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/gotham/src/middleware/session/backend/memory.rs b/gotham/src/middleware/session/backend/memory.rs index af0e72cc4..d7b2d2886 100644 --- a/gotham/src/middleware/session/backend/memory.rs +++ b/gotham/src/middleware/session/backend/memory.rs @@ -7,6 +7,7 @@ use futures::prelude::*; use linked_hash_map::LinkedHashMap; use log::trace; +use crate::state::State; use crate::middleware::session::backend::{Backend, NewBackend, SessionFuture}; use crate::middleware::session::{SessionError, SessionIdentifier}; @@ -80,6 +81,7 @@ impl NewBackend for MemoryBackend { impl Backend for MemoryBackend { fn persist_session( &self, + _: &State, identifier: SessionIdentifier, content: &[u8], ) -> Result<(), SessionError> { @@ -94,7 +96,7 @@ impl Backend for MemoryBackend { } } - fn read_session(&self, identifier: SessionIdentifier) -> Pin> { + fn read_session(&self, _: &State, identifier: SessionIdentifier) -> Pin> { match self.storage.lock() { Ok(mut storage) => match storage.get_refresh(&identifier.value) { Some(&mut (ref mut instant, ref value)) => { diff --git a/gotham/src/middleware/session/backend/mod.rs b/gotham/src/middleware/session/backend/mod.rs index a41f930b1..ee3a06f63 100644 --- a/gotham/src/middleware/session/backend/mod.rs +++ b/gotham/src/middleware/session/backend/mod.rs @@ -6,6 +6,7 @@ use std::pin::Pin; use futures::prelude::*; use crate::middleware::session::{SessionError, SessionIdentifier}; +use crate::state::State; /// A type which is used to spawn new `Backend` values. pub trait NewBackend: Sync + Clone + RefUnwindSafe { @@ -27,6 +28,7 @@ pub trait Backend: Send { /// Persists a session, either creating a new session or updating an existing session. fn persist_session( &self, + state: &State, identifier: SessionIdentifier, content: &[u8], ) -> Result<(), SessionError>; @@ -36,7 +38,7 @@ pub trait Backend: Send { /// The returned future will resolve to an `Option>` on success, where a value of /// `None` indicates that the session is not available for use and a new session should be /// established. - fn read_session(&self, identifier: SessionIdentifier) -> Pin>; + fn read_session(&self, state: &State, identifier: SessionIdentifier) -> Pin>; /// Drops a session from the underlying storage. fn drop_session(&self, identifier: SessionIdentifier) -> Result<(), SessionError>; diff --git a/gotham/src/middleware/session/mod.rs b/gotham/src/middleware/session/mod.rs index 55580cfe7..e8775da83 100644 --- a/gotham/src/middleware/session/mod.rs +++ b/gotham/src/middleware/session/mod.rs @@ -822,7 +822,7 @@ where ); self.backend - .read_session(id.clone()) + .read_session(&state, id.clone()) .then(move |r| self.load_session_into_state(state, id, r)) .and_then(move |state| chain(state)) .and_then(persist_session::) @@ -959,7 +959,7 @@ where let result = session_data .backend - .persist_session(identifier.clone(), slice); + .persist_session(&state, identifier.clone(), slice); match result { Ok(_) => { From 0c7e55af39805fdc5853b143c3d2101674396346 Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Fri, 4 Sep 2020 15:00:01 +0100 Subject: [PATCH 2/5] Added state to drop_session for Session backend. --- gotham/src/middleware/session/backend/memory.rs | 2 +- gotham/src/middleware/session/backend/mod.rs | 2 +- gotham/src/middleware/session/mod.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gotham/src/middleware/session/backend/memory.rs b/gotham/src/middleware/session/backend/memory.rs index d7b2d2886..d404b11b9 100644 --- a/gotham/src/middleware/session/backend/memory.rs +++ b/gotham/src/middleware/session/backend/memory.rs @@ -111,7 +111,7 @@ impl Backend for MemoryBackend { } } - fn drop_session(&self, identifier: SessionIdentifier) -> Result<(), SessionError> { + fn drop_session(&self, _: &State, identifier: SessionIdentifier) -> Result<(), SessionError> { match self.storage.lock() { Ok(mut storage) => { storage.remove(&identifier.value); diff --git a/gotham/src/middleware/session/backend/mod.rs b/gotham/src/middleware/session/backend/mod.rs index ee3a06f63..8810dc700 100644 --- a/gotham/src/middleware/session/backend/mod.rs +++ b/gotham/src/middleware/session/backend/mod.rs @@ -41,5 +41,5 @@ pub trait Backend: Send { fn read_session(&self, state: &State, identifier: SessionIdentifier) -> Pin>; /// Drops a session from the underlying storage. - fn drop_session(&self, identifier: SessionIdentifier) -> Result<(), SessionError>; + fn drop_session(&self, state: &State, identifier: SessionIdentifier) -> Result<(), SessionError>; } diff --git a/gotham/src/middleware/session/mod.rs b/gotham/src/middleware/session/mod.rs index e8775da83..0afda8c85 100644 --- a/gotham/src/middleware/session/mod.rs +++ b/gotham/src/middleware/session/mod.rs @@ -293,7 +293,7 @@ where state.put(SessionDropData { cookie_config: self.cookie_config, }); - self.backend.drop_session(self.identifier) + self.backend.drop_session(&state, self.identifier) } // Create a new, blank `SessionData` From 769e6a0f018c507d00617f1f1d3ada8826437261 Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Fri, 4 Sep 2020 22:17:55 +0100 Subject: [PATCH 3/5] Updated tests and formatting for Session Backend. --- .../src/middleware/session/backend/memory.rs | 14 ++++++++------ gotham/src/middleware/session/backend/mod.rs | 9 +++++++-- gotham/src/middleware/session/mod.rs | 19 +++++++++++-------- 3 files changed, 26 insertions(+), 16 deletions(-) diff --git a/gotham/src/middleware/session/backend/memory.rs b/gotham/src/middleware/session/backend/memory.rs index d404b11b9..8520fe526 100644 --- a/gotham/src/middleware/session/backend/memory.rs +++ b/gotham/src/middleware/session/backend/memory.rs @@ -7,9 +7,9 @@ use futures::prelude::*; use linked_hash_map::LinkedHashMap; use log::trace; -use crate::state::State; use crate::middleware::session::backend::{Backend, NewBackend, SessionFuture}; use crate::middleware::session::{SessionError, SessionIdentifier}; +use crate::state::State; /// Type alias for the `MemoryBackend` storage container. type MemoryMap = Mutex)>>; @@ -225,6 +225,7 @@ mod tests { fn memory_backend_test() { let new_backend = MemoryBackend::new(Duration::from_millis(100)); let bytes: Vec = (0..64).map(|_| rand::random()).collect(); + let state = State::new(); let identifier = SessionIdentifier { value: "totally_random_identifier".to_owned(), }; @@ -232,14 +233,14 @@ mod tests { new_backend .new_backend() .expect("can't create backend for write") - .persist_session(identifier.clone(), &bytes[..]) + .persist_session(&state, identifier.clone(), &bytes[..]) .expect("failed to persist"); let received = futures::executor::block_on( new_backend .new_backend() .expect("can't create backend for read") - .read_session(identifier.clone()), + .read_session(&state, identifier.clone()), ) .expect("no response from backend") .expect("session data missing"); @@ -251,6 +252,7 @@ mod tests { fn memory_backend_refresh_test() { let new_backend = MemoryBackend::new(Duration::from_millis(100)); let bytes: Vec = (0..64).map(|_| rand::random()).collect(); + let state = State::new(); let identifier = SessionIdentifier { value: "totally_random_identifier".to_owned(), }; @@ -264,11 +266,11 @@ mod tests { .expect("can't create backend for write"); backend - .persist_session(identifier.clone(), &bytes[..]) + .persist_session(&state, identifier.clone(), &bytes[..]) .expect("failed to persist"); backend - .persist_session(identifier2.clone(), &bytes2[..]) + .persist_session(&state, identifier2.clone(), &bytes2[..]) .expect("failed to persist"); { @@ -284,7 +286,7 @@ mod tests { ); } - futures::executor::block_on(backend.read_session(identifier.clone())) + futures::executor::block_on(backend.read_session(&state, identifier.clone())) .expect("failed to read session"); { diff --git a/gotham/src/middleware/session/backend/mod.rs b/gotham/src/middleware/session/backend/mod.rs index 8810dc700..6a50d0d48 100644 --- a/gotham/src/middleware/session/backend/mod.rs +++ b/gotham/src/middleware/session/backend/mod.rs @@ -38,8 +38,13 @@ pub trait Backend: Send { /// The returned future will resolve to an `Option>` on success, where a value of /// `None` indicates that the session is not available for use and a new session should be /// established. - fn read_session(&self, state: &State, identifier: SessionIdentifier) -> Pin>; + fn read_session(&self, state: &State, identifier: SessionIdentifier) + -> Pin>; /// Drops a session from the underlying storage. - fn drop_session(&self, state: &State, identifier: SessionIdentifier) -> Result<(), SessionError>; + fn drop_session( + &self, + state: &State, + identifier: SessionIdentifier, + ) -> Result<(), SessionError>; } diff --git a/gotham/src/middleware/session/mod.rs b/gotham/src/middleware/session/mod.rs index 0afda8c85..90dec18eb 100644 --- a/gotham/src/middleware/session/mod.rs +++ b/gotham/src/middleware/session/mod.rs @@ -231,12 +231,14 @@ impl SessionCookieConfig { /// # fn main() { /// # let backend = MemoryBackend::new(Duration::from_secs(1)); /// # let identifier = SessionIdentifier { value: "u0G6KdfckQgkV0qLANZjjNkEHBU".to_owned() }; -/// # let session = MySessionType { -/// # items: vec!["a".into(), "b".into(), "c".into()], -/// # }; /// # -/// # let bytes = bincode::serialize(&session).unwrap(); -/// # backend.persist_session(identifier.clone(), &bytes[..]).unwrap(); +/// # State::with_new(|state| { +/// # let session = MySessionType { +/// # items: vec!["a".into(), "b".into(), "c".into()], +/// # }; +/// # let bytes = bincode::serialize(&session).unwrap(); +/// # backend.persist_session(&state, identifier.clone(), &bytes[..]).unwrap(); +/// # }); /// # /// # let nm = NewSessionMiddleware::new(backend).with_session_type::(); /// # let nm = Arc::new(nm); @@ -1162,6 +1164,7 @@ mod tests { fn existing_session() { let nm = NewSessionMiddleware::default().with_session_type::(); let m = nm.new_middleware().unwrap(); + let mut state = State::new(); let identifier = m.random_identifier(); // 64 -> 512 bits = (85 * 6 + 2) @@ -1174,7 +1177,7 @@ mod tests { let bytes = bincode::serialize(&session).unwrap(); m.backend - .persist_session(identifier.clone(), &bytes) + .persist_session(&state, identifier.clone(), &bytes) .unwrap(); let received: Arc>> = Arc::new(Mutex::new(None)); @@ -1197,7 +1200,6 @@ mod tests { .boxed() }; - let mut state = State::new(); let mut headers = HeaderMap::new(); let cookie = Cookie::build("_gotham_session", identifier.value.clone()).finish(); headers.insert(COOKIE, cookie.to_string().parse().unwrap()); @@ -1216,8 +1218,9 @@ mod tests { Err((_, e)) => panic!("error: {:?}", e), } + let state = State::new(); let m = nm.new_middleware().unwrap(); - let bytes = futures::executor::block_on(m.backend.read_session(identifier)) + let bytes = futures::executor::block_on(m.backend.read_session(&state, identifier)) .unwrap() .unwrap(); let updated = bincode::deserialize::(&bytes[..]).unwrap(); From 15cb4a7a46e851fb88d5aad1359ff951bff3865f Mon Sep 17 00:00:00 2001 From: Stephen Wakely Date: Sun, 13 Sep 2020 11:26:12 +0100 Subject: [PATCH 4/5] Session backend functions to return Futures --- .../src/middleware/session/backend/memory.rs | 48 +++++++----- gotham/src/middleware/session/backend/mod.rs | 18 +++-- gotham/src/middleware/session/mod.rs | 74 ++++++++++--------- 3 files changed, 82 insertions(+), 58 deletions(-) diff --git a/gotham/src/middleware/session/backend/memory.rs b/gotham/src/middleware/session/backend/memory.rs index 8520fe526..86a5777f3 100644 --- a/gotham/src/middleware/session/backend/memory.rs +++ b/gotham/src/middleware/session/backend/memory.rs @@ -7,8 +7,10 @@ use futures::prelude::*; use linked_hash_map::LinkedHashMap; use log::trace; -use crate::middleware::session::backend::{Backend, NewBackend, SessionFuture}; -use crate::middleware::session::{SessionError, SessionIdentifier}; +use crate::middleware::session::backend::{ + Backend, GetSessionFuture, NewBackend, SetSessionFuture, +}; +use crate::middleware::session::SessionIdentifier; use crate::state::State; /// Type alias for the `MemoryBackend` storage container. @@ -84,11 +86,11 @@ impl Backend for MemoryBackend { _: &State, identifier: SessionIdentifier, content: &[u8], - ) -> Result<(), SessionError> { + ) -> Pin> { match self.storage.lock() { Ok(mut storage) => { storage.insert(identifier.value, (Instant::now(), Vec::from(content))); - Ok(()) + Box::pin(future::ok(())) } Err(PoisonError { .. }) => { unreachable!("session memory backend lock poisoned, HashMap panicked?") @@ -96,7 +98,7 @@ impl Backend for MemoryBackend { } } - fn read_session(&self, _: &State, identifier: SessionIdentifier) -> Pin> { + fn read_session(&self, _: &State, identifier: SessionIdentifier) -> Pin> { match self.storage.lock() { Ok(mut storage) => match storage.get_refresh(&identifier.value) { Some(&mut (ref mut instant, ref value)) => { @@ -111,11 +113,11 @@ impl Backend for MemoryBackend { } } - fn drop_session(&self, _: &State, identifier: SessionIdentifier) -> Result<(), SessionError> { + fn drop_session(&self, _: &State, identifier: SessionIdentifier) -> Pin> { match self.storage.lock() { Ok(mut storage) => { storage.remove(&identifier.value); - Ok(()) + future::ok(()).boxed() } Err(PoisonError { .. }) => { unreachable!("session memory backend lock poisoned, HashMap panicked?") @@ -230,11 +232,13 @@ mod tests { value: "totally_random_identifier".to_owned(), }; - new_backend - .new_backend() - .expect("can't create backend for write") - .persist_session(&state, identifier.clone(), &bytes[..]) - .expect("failed to persist"); + futures::executor::block_on( + new_backend + .new_backend() + .expect("can't create backend for write") + .persist_session(&state, identifier.clone(), &bytes[..]), + ) + .expect("failed to persist"); let received = futures::executor::block_on( new_backend @@ -265,13 +269,19 @@ mod tests { .new_backend() .expect("can't create backend for write"); - backend - .persist_session(&state, identifier.clone(), &bytes[..]) - .expect("failed to persist"); - - backend - .persist_session(&state, identifier2.clone(), &bytes2[..]) - .expect("failed to persist"); + futures::executor::block_on(backend.persist_session( + &state, + identifier.clone(), + &bytes[..], + )) + .expect("failed to persist"); + + futures::executor::block_on(backend.persist_session( + &state, + identifier2.clone(), + &bytes2[..], + )) + .expect("failed to persist"); { let mut storage = backend.storage.lock().expect("couldn't lock storage"); diff --git a/gotham/src/middleware/session/backend/mod.rs b/gotham/src/middleware/session/backend/mod.rs index 6a50d0d48..5f9cc058d 100644 --- a/gotham/src/middleware/session/backend/mod.rs +++ b/gotham/src/middleware/session/backend/mod.rs @@ -17,8 +17,11 @@ pub trait NewBackend: Sync + Clone + RefUnwindSafe { fn new_backend(&self) -> anyhow::Result; } -/// Type alias for the trait objects returned by `Backend`. -pub type SessionFuture = dyn Future>, SessionError>> + Send; +/// Type alias for the trait objects that returned by `Backend`. +pub type GetSessionFuture = dyn Future>, SessionError>> + Send; + +/// Type alias for the trait objects that set the session in the `Backend`. +pub type SetSessionFuture = dyn Future> + Send; /// A `Backend` receives session data and stores it, and recalls the session data subsequently. /// @@ -31,20 +34,23 @@ pub trait Backend: Send { state: &State, identifier: SessionIdentifier, content: &[u8], - ) -> Result<(), SessionError>; + ) -> Pin>; /// Retrieves a session from the underlying storage. /// /// The returned future will resolve to an `Option>` on success, where a value of /// `None` indicates that the session is not available for use and a new session should be /// established. - fn read_session(&self, state: &State, identifier: SessionIdentifier) - -> Pin>; + fn read_session( + &self, + state: &State, + identifier: SessionIdentifier, + ) -> Pin>; /// Drops a session from the underlying storage. fn drop_session( &self, state: &State, identifier: SessionIdentifier, - ) -> Result<(), SessionError>; + ) -> Pin>; } diff --git a/gotham/src/middleware/session/mod.rs b/gotham/src/middleware/session/mod.rs index 90dec18eb..f220436e6 100644 --- a/gotham/src/middleware/session/mod.rs +++ b/gotham/src/middleware/session/mod.rs @@ -19,7 +19,7 @@ use serde::{Deserialize, Serialize}; use super::cookie::CookieParser; use super::{Middleware, NewMiddleware}; -use crate::handler::{HandlerError, HandlerFuture}; +use crate::handler::{HandlerError, HandlerFuture, HandlerResult}; use crate::helpers::http::response::create_empty_response; use crate::state::{self, FromState, State, StateData}; @@ -27,7 +27,7 @@ mod backend; mod rng; pub use self::backend::memory::MemoryBackend; -pub use self::backend::{Backend, NewBackend}; +pub use self::backend::{Backend, GetSessionFuture, NewBackend, SetSessionFuture}; const SECURE_COOKIE_PREFIX: &str = "__Secure-"; const HOST_COOKIE_PREFIX: &str = "__Host-"; @@ -237,7 +237,12 @@ impl SessionCookieConfig { /// # items: vec!["a".into(), "b".into(), "c".into()], /// # }; /// # let bytes = bincode::serialize(&session).unwrap(); -/// # backend.persist_session(&state, identifier.clone(), &bytes[..]).unwrap(); +/// # futures::executor::block_on(backend.persist_session( +/// # &state, +/// # identifier.clone(), +/// # &bytes[..] +/// # )) +/// # .unwrap(); /// # }); /// # /// # let nm = NewSessionMiddleware::new(backend).with_session_type::(); @@ -291,7 +296,10 @@ where /// Discards the session, invalidating it for future use and removing the data from the /// `Backend`. // TODO: Add test case that covers this. - pub fn discard(self, state: &mut State) -> Result<(), SessionError> { + pub fn discard( + self, + state: &mut State, + ) -> Pin>>> { state.put(SessionDropData { cookie_config: self.cookie_config, }); @@ -867,7 +875,7 @@ where fn persist_session( (mut state, mut response): (State, Response), -) -> impl Future), (State, HandlerError)>> +) -> Pin), (State, HandlerError)>> + Send>> where T: Default + Serialize + for<'de> Deserialize<'de> + Send + 'static, { @@ -878,7 +886,7 @@ where state::request_id(&state) ); reset_cookie(&mut response, session_drop_data); - return future::ok((state, response)).right_future(); + return Box::pin(future::ok((state, response))); } None => { trace!( @@ -895,14 +903,12 @@ where } match session_data.state { - SessionDataState::Dirty => { - write_session(state, response, session_data).left_future() - } - SessionDataState::Clean => future::ok((state, response)).right_future(), + SessionDataState::Dirty => write_session(state, response, session_data), + SessionDataState::Clean => Box::pin(future::ok((state, response))), } } // Session was discarded with `SessionData::discard`, or otherwise removed - None => future::ok((state, response)).right_future(), + None => Box::pin(future::ok((state, response))), } } @@ -937,7 +943,7 @@ fn write_session( state: State, response: Response, session_data: SessionData, -) -> impl Future), (State, HandlerError)>> +) -> Pin + Send>> where T: Default + Serialize + for<'de> Deserialize<'de> + Send + 'static, { @@ -952,33 +958,33 @@ where let response = create_empty_response(&state, StatusCode::INTERNAL_SERVER_ERROR); - return future::ok((state, response)); + return Box::pin(future::ok((state, response))); } }; let identifier = session_data.identifier; let slice = &bytes[..]; - let result = session_data + session_data .backend - .persist_session(&state, identifier.clone(), slice); - - match result { - Ok(_) => { - trace!( - "[{}] persisted session ({}) successfully", - state::request_id(&state), - identifier.value - ); + .persist_session(&state, identifier.clone(), slice) + .then(move |result| match result { + Ok(_) => { + trace!( + "[{}] persisted session ({}) successfully", + state::request_id(&state), + identifier.value + ); - future::ok((state, response)) - } - Err(_) => { - let response = create_empty_response(&state, StatusCode::INTERNAL_SERVER_ERROR); + future::ok((state, response)) + } + Err(_) => { + let response = create_empty_response(&state, StatusCode::INTERNAL_SERVER_ERROR); - future::ok((state, response)) - } - } + future::ok((state, response)) + } + }) + .boxed() } impl SessionMiddleware @@ -1176,9 +1182,11 @@ mod tests { }; let bytes = bincode::serialize(&session).unwrap(); - m.backend - .persist_session(&state, identifier.clone(), &bytes) - .unwrap(); + futures::executor::block_on( + m.backend + .persist_session(&state, identifier.clone(), &bytes), + ) + .unwrap(); let received: Arc>> = Arc::new(Mutex::new(None)); let r = received.clone(); From 204cc251839aef1b0894a8635e0a59e793067648 Mon Sep 17 00:00:00 2001 From: Dominic Date: Sat, 8 May 2021 16:20:52 +0200 Subject: [PATCH 5/5] fix clippy lint --- gotham/src/middleware/session/backend/memory.rs | 3 +-- gotham/src/middleware/session/mod.rs | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/gotham/src/middleware/session/backend/memory.rs b/gotham/src/middleware/session/backend/memory.rs index 13da50a90..e4ac2f67c 100644 --- a/gotham/src/middleware/session/backend/memory.rs +++ b/gotham/src/middleware/session/backend/memory.rs @@ -197,7 +197,6 @@ fn cleanup_once( mod tests { use super::*; - #[test] fn cleanup_test() { let mut storage = LinkedHashMap::new(); @@ -312,4 +311,4 @@ mod tests { ); } } -} \ No newline at end of file +} diff --git a/gotham/src/middleware/session/mod.rs b/gotham/src/middleware/session/mod.rs index de396dbbf..9bc99106b 100644 --- a/gotham/src/middleware/session/mod.rs +++ b/gotham/src/middleware/session/mod.rs @@ -871,7 +871,7 @@ where fn persist_session( (mut state, mut response): (State, Response), -) -> Pin), (State, HandlerError)>> + Send>> +) -> Pin + Send>> where T: Default + Serialize + for<'de> Deserialize<'de> + Send + 'static, {