[SessionRepositoryFilter] double save session to repository

[edgar-dev20]

Expected Behavior

When we use sessionRepositryFilter we find the session once and save it to the repository once at the end of the request

Current Behavior
SessionRepositryFilter at the end of the request find the session 2 times and save it twice. It's all about

spring-session/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java

Line 181 in c98a7be

this.request.commitSession();

Why is this necessary if after all the filters we still make a commit?

spring-session/spring-session-core/src/main/java/org/springframework/session/web/http/SessionRepositoryFilter.java

Line 146 in c98a7be

wrappedRequest.commitSession();

Maybe just use the final commit?

[buzzerrookie]

As far as I know, the first commitSession is necessary, as it is intended for committed response. When a response is committed, the first commitSession is called, and the cookie header and other headers are written to the response. Because the response headers can not be modified once a response is commited, we need to do these before the response is committed.

[edgar-dev20]

As far as I know, the first commitSession is necessary, as it is intended for committed response. When a response is committed, the first commitSession is called, and the cookie header and other headers are written to the response. Because the response headers can not be modified once a response is commited, we need to do these before the response is committed.

how are headers and cookies related to the session object in storage?

[buzzerrookie]

In commitSession method of class SessionRepositoryRequestWrapper, HttpSessionIdResolver is used to set session id via cookie or header. This happens before the response is committed.

[ravenadesk]

@buzzerrookie
I am not sure if it is a bug, but I am facing the inconsistency between the two saves when using redis with replica.

In our project, we are using redis as the storage, there is one master and two replica.
When I was testing /health endpoint with a little load, sometimes the second save cannot get the right session status (taking it as not stored in redis) throwing IllegalStateException

@Override
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();
}

I have tried adding a conditional break point at the line before if condition, and let it pause a little while when key does not exist. And it turns out no problem(key exists, no exception). I have also tried that remove all my replica of redis, and there is no problem at all.
So I am wondering is this twice save patten conflict against replica synchronization of redis.

[marcusdacoregio]

Hi, @ravenadesk. I think your issue might be related to #2021

[ravenadesk]

@marcusdacoregio
Thank you.
But my problem is that there exists the possibility to not able to get session from replica after just set it.
I have adjusted my config to not use read replica.

[marcusdacoregio]

Hi, @edgar-dev20. Can you provide an example where this might be causing problems for you? We have to make sure that we commit the session when response is committed because after that we might see errors like Cannot create a session after the response has been committed.

[spring-projects-issues]

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

[spring-projects-issues]

Closing due to lack of requested feedback. If you would like us to look at this issue, please provide the requested information and we will re-open the issue.