id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
6984	Session Data Race Condition Leading to Dropped Sessions	Simon Blanchard	Malcolm Tredinnick	"Symptom:

SESSION_SAVE_EVERY_REQUEST = True

SESSION_ENGINE = 'django.contrib.sessions.backends.file'

With the settings as above we observe that the session cookie changes on every request.

Cause:

The save() method calls the load() method after it has truncated the file. 

The !SessionStore.save() method writes back the cached session dict to storage. If the session dict (!SessionBase._session_cache) is not already cached, a cache is attempted via accessing the session._session attribute. That is, for uncached sessions the save() method will (indirectly) call the load method. The problem (in the file backend) is that this happens after the session file store has been open with 'wb' - i.e. truncated. So it loads an empty file, triggering a !SuspiciousOperation exception form !SessionBase.decode() which cause a new session key to be generated and sent, which means a new file and hence all the session data gone.

Solution:

Make sure the session is cached before the store is written in the save method. One way to do this is in the attached patch.

Discussion:

This was a devil to track down. My client's log in sessions where getting dropped randomly. I set up a duplicate test server and (of course) the problem did not manifest itself. I instrumented the code with debug logging - the problem went away. Don't you just love it when that happens. It does at least point to a race/timing problem. Finally, I inserted flocks around access to the session file store. This lead to the response hanging as the load method (called via save()) could never lock the file since the save() method had it locked and was not finished with it yet.

I was using the database as session storage and seeing the same errors. I switched to the file backend to try to track the problem. So this problem seems to be endemic to the session backends. Though it is much easier to track on the file backend. It is also, because it is a timing issue, dependant on the server setup hw/sw so it may not manifest everywhere.

Other notes:

The attached patch also corrects a spelling error in the comments of the Session middleware

This error message: raise !SuspiciousOperation(""User tampered with session cookie."") from sessions/backends/base.py is mis-leading. Actually the exposed tamper is with the session data as stored on the server not with the cookie per se. Of course, to do that someone would have to have access to the session store. Perhaps a better message would be ""Invalid session data""







"		closed	contrib.sessions	dev		fixed		bnomis@…	Accepted	1	0	0	1	0	0