fix session saving and loading data race (#36)

* fix session saving and loading data race

This is a temporary fix for a data race that occurs when a session is
saved and loaded at the same time. The result of this race is that stale
data can be loaded into memory, resulting in potential data loss where
multiple requests using the same session are issued simultaneously.

Here we address this by holding a lock for the duration of the request.
This solution is only temporary, as the underlying session
implementation can instead be modified to ensure safe access between the
save and load await points.

This supercedes #35.

CHANGELOG.md

@@ -1,5 +1,9 @@
 # Unreleased
 
+# 0.2.4
+
+- Fix session saving and loading potential data race. #36
+
 # 0.2.3
 
 - Fix setting of modified in `replace_if_equal`.

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "tower-sessions"
 description = "🥠 Sessions as a `tower` and `axum` middleware."
-version = "0.2.3"
+version = "0.2.4"
 edition = "2021"
 authors = ["Max Countryman <hello@maxcountryman.com>"]
 license = "MIT"
@@ -23,7 +23,7 @@ sqlite-store = ["sqlx/sqlite", "sqlx-store"]
 postgres-store = ["sqlx/postgres", "sqlx-store"]
 mysql-store = ["sqlx/mysql", "sqlx-store"]
 moka-store = ["moka"]
-tokio = ["dep:tokio"]
+#tokio = ["dep:tokio"]
 
 [dependencies]
 async-trait = "0.1.73"
@@ -41,6 +41,9 @@ tower-layer = "0.3.2"
 tower-service = "0.3.2"
 uuid = { version = "1.4.1", features = ["v4", "serde"] }
 
+# This is temporarily required; move back to optional once session lock can be removed.
+tokio = { version = "1.32.0", features = ["full"] }
+
 axum-core = { optional = true, version = "0.3.4" }
 fred = { optional = true, version = "6.3.2" }
 rmp-serde = { optional = true, version = "1.1.2" }
@@ -51,7 +54,6 @@ sqlx = { optional = true, version = "0.7.2", features = [
     "uuid",
     "runtime-tokio",
 ] }
-tokio = { optional = true, version = "1.32.0", features = ["full"] }
 moka = { optional = true, version = "0.12.0", features = ["future"] }
 
 [dev-dependencies]

README.md

@@ -53,7 +53,7 @@ To use the crate in your project, add the following to your `Cargo.toml` file:
 
 ```toml
 [dependencies]
-tower-sessions = "0.2.3"
+tower-sessions = "0.2.4"
 ```
 
 ## 🤸 Usage

src/mongodb_store.rs

@@ -62,6 +62,7 @@ impl MongoDBStore {
         }
     }
 
+    //#[cfg(feature = "tokio")]
     /// This function will keep running indefinitely, deleting expired documents
     /// and then waiting for the specified period before deleting again.
     ///
@@ -94,7 +95,6 @@ impl MongoDBStore {
     /// );
     /// # })
     /// ```
-    #[cfg(feature = "tokio")]
     pub async fn continuously_delete_expired(
         self,
         period: tokio::time::Duration,

src/service.rs

@@ -2,11 +2,13 @@
 use std::{
     future::Future,
     pin::Pin,
+    sync::Arc,
     task::{Context, Poll},
 };
 
 use http::{Request, Response};
 use time::Duration;
+use tokio::sync::Mutex;
 use tower_cookies::{cookie::SameSite, Cookie, CookieManager, Cookies};
 use tower_layer::Layer;
 use tower_service::Service;
@@ -22,6 +24,11 @@ pub struct SessionManager<S, Store: SessionStore> {
     inner: S,
     session_store: Store,
     cookie_config: CookieConfig,
+
+    // TODO: This is a stopgap measure to ensure correctness. However, it should not be required
+    // and will be removed once the underlying session implementation is modified to ensure
+    // correct concurrent access.
+    session_lock: Arc<Mutex<()>>,
 }
 
 impl<S, Store: SessionStore> SessionManager<S, Store> {
@@ -31,6 +38,7 @@ impl<S, Store: SessionStore> SessionManager<S, Store> {
             inner,
             session_store,
             cookie_config,
+            session_lock: Default::default(),
         }
     }
 }
@@ -56,10 +64,13 @@ where
     fn call(&mut self, mut req: Request<ReqBody>) -> Self::Future {
         let session_store = self.session_store.clone();
         let cookie_config = self.cookie_config.clone();
+        let session_lock = self.session_lock.clone();
 
         let clone = self.inner.clone();
         let mut inner = std::mem::replace(&mut self.inner, clone);
         Box::pin(async move {
+            let _session_guard = session_lock.lock().await;
+
             let cookies = req
                 .extensions()
                 .get::<Cookies>()
@@ -282,6 +293,7 @@ impl<S, Store: SessionStore> Layer<S> for SessionManagerLayer<Store> {
             inner,
             session_store: self.session_store.clone(),
             cookie_config: self.cookie_config.clone(),
+            session_lock: Default::default(),
         };
 
         CookieManager::new(session_manager)

src/sqlx_store/mysql_store.rs

@@ -79,7 +79,7 @@ impl MySqlStore {
         Ok(())
     }
 
-    #[cfg(feature = "tokio")]
+    //#[cfg(feature = "tokio")]
     /// This function will keep running indefinitely, deleting expired rows and
     /// then waiting for the specified period before deleting again.
     ///

src/sqlx_store/postgres_store.rs

@@ -91,7 +91,7 @@ impl PostgresStore {
         Ok(())
     }
 
-    #[cfg(feature = "tokio")]
+    //#[cfg(feature = "tokio")]
     /// This function will keep running indefinitely, deleting expired rows and
     /// then waiting for the specified period before deleting again.
     ///

src/sqlx_store/sqlite_store.rs

@@ -78,7 +78,7 @@ impl SqliteStore {
         Ok(())
     }
 
-    #[cfg(feature = "tokio")]
+    //#[cfg(feature = "tokio")]
     /// This function will keep running indefinitely, deleting expired rows and
     /// then waiting for the specified period before deleting again.
     ///