prepare_generate to set new session id if old id does not exists (#21)

* prepare_generate to set new session id if old id does not exists

* fixed inconsistent session behaviour in redis store during update

CHANGELOG.md

@@ -1,5 +1,17 @@
 # Changelog
 
+## [0.5.5] - 2024-02-16
+
+### Fixed
+
+- Inconsistent session expiration behavior in Redis UPDATE script
+- Race conditions and atomicity issues in Redis UPDATE_WITH_RENAME script
+
+## [0.5.4] - 2024-02-16
+
+### Changed
+- `prepare_regenerate()` will rename a session id if it exists, if not, a new session id is set instead.
+
 ## [0.5.3] - 2024-02-08
 
 ### Added

Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "ruts"
 description = "A middleware for tower sessions"
-version = "0.5.3"
+version = "0.5.5"
 edition = "2021"
 rust-version = "1.75.0"
 authors = ["Jimmie Lovell <jimmieomlovell@gmail.com>"]

README.md

@@ -23,7 +23,7 @@ Add the following to your `Cargo.toml`:
 
 ```toml
 [dependencies]
-ruts = "0.5.3"
+ruts = "0.5.5"
 ```
 
 ## Quick Start

examples/axum.rs

@@ -33,6 +33,26 @@ struct AppSession {
 
 fn routes() -> Router {
     Router::new()
+        .route(
+            "/prepare_and_update",
+            get(|session: RedisSession| async move {
+                let app_session: AppSession = AppSession {
+                    user: Some(User {
+                        id: 34895634,
+                        name: String::from("John Doe"),
+                    }),
+                    ip: Some(IpAddr::from(Ipv4Addr::new(192, 168, 0, 1))),
+                    theme: Some(Theme::Dark),
+                };
+
+                session.prepare_regenerate();
+                session
+                    .update("app", &app_session, None)
+                    .await
+                    .map_err(|e| e.to_string())
+                    .unwrap();
+            }),
+        )
         .route(
             "/insert",
             get(|session: RedisSession| async move {
@@ -125,6 +145,6 @@ async fn main() {
         .layer(CookieManagerLayer::new());
 
     // Run the server
-    let listener = tokio::net::TcpListener::bind("0.0.0.0:3000").await.unwrap();
+    let listener = tokio::net::TcpListener::bind("0.0.0.0:9000").await.unwrap();
     axum::serve(listener, app).await.unwrap();
 }

src/session/mod.rs

@@ -159,7 +159,7 @@ where
     /// ```
     #[tracing::instrument(
         name = "session-store: inserting field-value",
-        skip(self, field, value)
+        skip(self, field, value, field_expire)
     )]
     pub async fn insert<T>(&self, field: &str, value: &T, field_expire: Option<i64>) -> Result<bool>
     where
@@ -245,7 +245,7 @@ where
     ///     let updated = session.update("app", &app, Some(5)).await.unwrap();
     /// }
     /// ```
-    #[tracing::instrument(name = "session-store: updating field", skip(self, field, value))]
+    #[tracing::instrument(name = "session-store: updating field", skip(self, field, value, field_expire))]
     pub async fn update<T>(&self, field: &str, value: &T, field_expire: Option<i64>) -> Result<bool>
     where
         T: Send + Sync + Serialize,
@@ -454,7 +454,7 @@ where
     }
 
     /// Prepares a new session ID to be used in the next store operation.
-    /// The new ID will be used to rename the current session when the next
+    /// The new ID will be used to rename the current session (if it exists) when the next
     /// insert or update operation is performed.
     ///
     /// # Example
@@ -471,9 +471,13 @@ where
     /// }
     /// ```
     pub fn prepare_regenerate(&self) -> Id {
-        let new_id = Id::default();
-        self.inner.set_pending_id(Some(new_id));
-        new_id
+        if self.id().is_none() {
+            self.inner.get_or_set_id()
+        } else {
+            let new_id = Id::default();
+            self.inner.set_pending_id(Some(new_id));
+            new_id
+        }
     }
 
     /// Returns the session ID, if it exists.

src/store/redis/lua.rs

@@ -21,6 +21,7 @@ pub(crate) static INSERT_SCRIPT: &str = r#"
             redis.call('HEXPIRE', key, tonumber(field_seconds), 'FIELDS', 1, field)
         end
     end
+
     return inserted
 "#;
 
@@ -31,14 +32,16 @@ pub(crate) static UPDATE_SCRIPT: &str = r#"
     local key_seconds = tonumber(ARGV[3])
     local field_seconds = ARGV[4]
 
+    local exists = redis.call('EXISTS', key)
     local updated = redis.call('HSET', key, field, value)
-    if updated == 1 then
-        redis.call('EXPIRE', key, key_seconds)
-        if field_seconds ~= '' then
-            redis.call('HEXPIRE', key, tonumber(field_seconds), 'FIELDS', 1, field)
-        end
+
+    redis.call('EXPIRE', key, key_seconds)
+
+    if field_seconds ~= '' then
+        redis.call('HEXPIRE', key, tonumber(field_seconds), 'FIELDS', 1, field)
     end
-    return updated
+
+    return updated > 0 or exists == 1
 "#;
 
 pub(crate) static INSERT_WITH_RENAME_SCRIPT: &str = r#"
@@ -49,24 +52,19 @@ pub(crate) static INSERT_WITH_RENAME_SCRIPT: &str = r#"
     local key_seconds = tonumber(ARGV[3])
     local field_seconds = ARGV[4]
 
-    local exists = redis.call('EXISTS', old_key)
-    if exists == 0 then
-        return 0
+    local renamed = redis.call('RENAMENX', old_key, new_key)
+    if renamed == 0 then
+        return 0  -- Either old_key doesn't exist or new_key already exists
     end
 
-    local new_exists = redis.call('EXISTS', new_key)
-    if new_exists == 1 then
-        return 0
-    end
+    -- Now we own new_key exclusively
+    local inserted = redis.call('HSETNX', new_key, field, value)
 
-    local inserted = redis.call('HSETNX', old_key, field, value)
-    if inserted == 1 then
-        redis.call('RENAMENX', old_key, new_key)
-        redis.call('EXPIRE', new_key, key_seconds)
-        if field_seconds ~= '' then
-            redis.call('HEXPIRE', new_key, tonumber(field_seconds), 'FIELDS', 1, field)
-        end
+    redis.call('EXPIRE', new_key, key_seconds)
+    if field_seconds ~= '' then
+        redis.call('HEXPIRE', new_key, tonumber(field_seconds), 'FIELDS', 1, field)
     end
+
     return inserted
 "#;
 
@@ -78,26 +76,20 @@ pub(crate) static UPDATE_WITH_RENAME_SCRIPT: &str = r#"
     local key_seconds = tonumber(ARGV[3])
     local field_seconds = ARGV[4]
 
-    local exists = redis.call('EXISTS', old_key)
-    if exists == 0 then
-        return 0
+    local renamed = redis.call('RENAMENX', old_key, new_key)
+    if renamed == 0 then
+        return 0  -- Either old_key doesn't exist or new_key already exists
     end
 
-    local new_exists = redis.call('EXISTS', new_key)
-    if new_exists == 1 then
-        return 0
-    end
+    -- Now we own new_key exclusively
+    local updated = redis.call('HSET', new_key, field, value)
 
-    -- Update the field
-    local updated = redis.call('HSET', old_key, field, value)
-    if updated == 1 then
-        redis.call('RENAMENX', old_key, new_key)
-        redis.call('EXPIRE', new_key, key_seconds)
-        if field_seconds ~= '' then
-            redis.call('HEXPIRE', new_key, tonumber(field_seconds), 'FIELDS', 1, field)
-        end
+    redis.call('EXPIRE', new_key, key_seconds)
+    if field_seconds ~= '' then
+        redis.call('HEXPIRE', new_key, tonumber(field_seconds), 'FIELDS', 1, field)
     end
-    return updated
+
+    return 1
 "#;
 
 pub(crate) const RENAME_SCRIPT: &str = r#"
@@ -109,5 +101,6 @@ pub(crate) const RENAME_SCRIPT: &str = r#"
     if renamed == 1 then
         redis.call('EXPIRE', new_key, seconds)
     end
+
     return renamed
 "#;