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

Reconsider handling of SessionRepository#save for invalidated sessions #1277

Open
vpavic opened this issue Dec 4, 2018 · 2 comments
Open
Labels

Comments

@vpavic
Copy link
Contributor

vpavic commented Dec 4, 2018

At present, the SessionRepository implementations are silent when an invalid session is passed to #save operation. This can have unexpected consequences in further processing of HTTP request, since it operates under assumption that the saved session is still valid - i.e. there's no difference in #save when passed in session is valid vs invalid.

One option could be (depending on the nature of the underlying data store) to do a read before saving a session, and throw an error if session is missing/invalid.

We could look at WebSession#save for inspiration - see #1135.

/cc @rwinch @jxblum @gregturn

@chrisgolle
Copy link

chrisgolle commented May 31, 2022

Just wanted to comment that I see opposite behaviour using RedisSessionRepository and would prefer it were silent haha. Rarely, i have requests come in with an existing session and they are unlucky enough to be gone from Redis at the time the session is saved, triggering the IllegalStateException below.

	public void save(RedisSession session) {
		if (!session.isNew) {
			String key = getSessionKey(session.hasChangedSessionId() ? session.originalSessionId : session.getId());
			Boolean sessionExists = this.sessionRedisOperations.hasKey(key);
			if (sessionExists == null || !sessionExists) {
				throw new IllegalStateException("Session was invalidated");
			}
		}
		session.save();
}`

@yu-shiba
Copy link

@chrisgolle, Throwing an IllegalStateException can be avoided by extending the expiration time of Session Storage to avoid unlucky cases.
I workaround this in #2786 (comment) by delegating RedisTemplate and adding a session timeout to the expireAt.

@vpavic Is there a possibility to avoid this behavior in the implementation and is there room to consider it?

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

No branches or pull requests

5 participants