Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Session backends are not interchangeable #54

Open
MOZGIII opened this issue Dec 16, 2022 · 0 comments
Open

Session backends are not interchangeable #54

MOZGIII opened this issue Dec 16, 2022 · 0 comments

Comments

@MOZGIII
Copy link

MOZGIII commented Dec 16, 2022

I've tried using MemoryStore, but when I try to load the session and then alter it and finally save it the result of the store_session call is None, even though the session is not empty.

The way I handle it is as follows:

        let jar = match session_cookie {
            Some(session_cookie) => jar.add(Cookie::build(SESSION_COOKIE_NAME, session_cookie).finish()),
            None => jar.remove(Cookie::named(SESSION_COOKIE_NAME)),
        };

Apparently, this crate expects me to do this instead:

        let jar = match session_cookie {
            Some(session_cookie) => jar.add(Cookie::build(SESSION_COOKIE_NAME, session_cookie).finish()),
            None => { /* do nothing */ },
        };

The problem with this API is that the library does not provide itself control over how the cookie actually has to be set. Meaning, the library must be aware that some cookie (either the whole set of values, encoded somehow, or a key into the database with the sessions) must be set, and at some point, that cookie has to be updated (almost never when the id is stored, and a lot when the session value is stored) or removed (in both cases, when the session is destroyed). The library can thus expect to return the effective cookie value for the server to set on the client; the server handling the cookie can then check if the cookie has to be set.

I propose that we change the API as follows:

async fn store_session(...) -> Result<CookieAction>;
enum CookieAction {
  /// Set the cookie with this new value.
  Set(String),
  /// Remove the cookie.
  Remove,
  /// No action is required (the cookie value should not change).
  NoAction,
}

Or at least remap the semantics of what None encodes currently to make it so it means "remove the cookie value on the client side".


Without this change, it seems I wouldn't be able to swap between the Memory and Cookie stores - as I'd have to knowledgeably choose whether to set the cookie or not depending on the backend I use. I know I'd have to set the cookie every time the session values change when using the CookieStore backend.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant