session.flush should not delete the old session

[Glenn Maynard]

Flushing and cycling the session should empty the data in the session and create a new key, but should not delete the old key.

Scenario:

1: JS kicks off a periodic AJAX request to update something, which is delayed in transit.

2: User submits an AJAX login form, which calls auth.login, calling session.flush or session.cycle_key. The AJAX response sets a new session cookie for the user.

3: The async request from #1 makes it to the server. This still has the old cookie, since it started before #2 finished. contrib.session doesn't recognize the cookie, since the previous request deleted it. It thinks it's an expired or corrupt session cookie, and flushes the session again.

#2 logs the user in, then #3 logs the user back out. (I've seen this happen even without AJAX logins, when using django.views.static.serve in development.)

session.flush should leave the old session in the database, and just clear its data. That way, when #3 comes around, it won't be an unrecognized session, and it won't trigger a session flush. Let the old session row expire on its own, like any idle session.

This doesn't change the definition of the function: "Removes the current session data from the database and regenerates the key."

This patch also fixes and tests session.cycle_key() raising an error if no session already existed; accessing self._session_cache raises AttributeError. This was triggering while I was writing the main test.

[Glenn Maynard]

As a followup: this will still fail if the delayed request modifies the session, since it'll refresh the cookie.

A fix would be to update the cookie only when the session hasn't been updated in over some timeout (say, a minute); this is long enough to avoid this race condition. This would have the nice side benefit of not sending a Set-Cookie header for each and every request that modifies the session.

It's harder to implement cleanly, though, since the session rows hold an expiry date, not a last-saved date. You can't reliably derive one from the other after the fact, since the session expiry setting might have changed. I'll leave this for further discussion.

[Glenn Maynard]

The test case should probably use django.test.client, but until there's some indication of interest, I'll hold off on further updates.

[Aymeric Augustin]

Change UI/UX from NULL to False.

[Aymeric Augustin]

Change Easy pickings from NULL to False.

[Tomek Paczkowski]

Patch no longer applies. As the ticket is 5 years old it would be worth checking that the problems still occurs.

[Ramon Saraiva]

Replying to Tomek Paczkowski:

Patch no longer applies. As the ticket is 5 years old it would be worth checking that the problems still occurs.

This seems to be currently happening in our production environment and it was extremely difficult to reproduce. I'll try to explain our use case as an attempt to revive this issue.

We have a few ajax requests that are initiated simultaneously, and one of them is responsible for logging in a user. Depending on how requests are distributed in nodes/processes/threads, if any of the requests that have the same session id cookie get processed a bit after the one that logs the user in, Django will generate a response setting the session id cookie to an empty value. This ends up making the application lose the login state and any data that was previously written to the session.

When Django is logging the user in, a new session id is generated and the data is moved over to this new location. Everything works well despite the fact that Django deletes the old session id from the session engine (i.e.: cache), making any attempt to load data from the old session id result on a new session id or simply a blank session id set cookie.

To replicate that, I created 3 views:

• GET / returns an empty HttpResponse (this will be used to simulate the blank session id set cookie response)
• GET /session/ adds a simple value to request.session and generates a sessionid
• POST /login/ logs an user in

1. Create a single request to /session/ generating a new sessionid cookie
2. Create multiple async fetches to / while having 1 of them posting to /login/
3. Anytime that one of the / requests get processed after /login/ was processed, your sessionid cookie is gone
4. If you delay the /login/ request, a new sessionid is received and everything works as usual

This can potentially be fixed in the application itself, by removing concurrent requests that happen in the same time that someone is logging in, but there might be ways to avoid that within the framework, maybe avoiding to delete the session key from the engine as soon as a new session key is generated. Or allowing developers to customize whether they want the previous session to actually be removed from the engine, allowing them to simply let sessions expire.

Warnings would also be helpful to speed up the debugging process of something like this.

Would love to know what you all think about this,
Thanks!