SESSION_SAVE_EVERY_REQUEST Appears to have broke in 1.8.4

[Damon Timm]

After upgrading to Django 1.8.4 the request.session.session_key value always returns None for a non-authenticated user even when SESSION_SAVE_EVERY_REQUEST is set to True.

I was able to reproduce this by creating a new project and app using a very basic view:

# settings.py
SESSION_SAVE_EVERY_REQUEST = True

# views.py
from django.http import HttpResponse

def index(request):
    return HttpResponse('Session Key is: %s' % request.session.session_key)

#urls.py
from django.conf.urls import url

from test_app import views

urlpatterns = [
    url(r'^$', views.index, name="index"),
]

In Django <=1.8.3 this view will start to return a session key after the first page load (you have to refresh is a second time, of course, but you will get a key!).

But with Django 1.8.4 it continues to return None even after refreshing the browser. (Be sure to remove your site cookies during testing because the key can persist between downgrading and upgrading of Django.)

First time submitting a ticket ... I hope this is helpful.

[Tim Graham]

This is due to the security fix in 8cc41ce7a7a8f6bebfdd89d5ab276cd0109f4fc5. Do you really require the creation of empty sessions (and do you accept the DoS possibilities that it entails)? We should at least update the documentation to better clarify the expected behavior.

[Damon Timm]

Well ... I am certainly no expert on security and would defer to a wiser cohort! Smile.

From my standpoint, it seems like the expected behavior of SESSION_SAVE_EVERY_REQUEST has changed. The outcome it was producing was what I had come to expect. By adding the and not Empty in that patch doesn't it kind of negate the setting itself (because I thought the setting was supposed to force it to save even if it was empty) ... or am I misunderstanding that?

[Tim Graham]

I'm not sure what the intended use case of SESSION_SAVE_EVERY_REQUEST is -- that doesn't seem to be described in the original commit where it was added: 3895a825a9696b58db1a0a2f6f30b1b023d58050. Could you describe why enabling the setting is useful for you?

[Carl Meyer]

The one time I used SESSION_SAVE_EVERY_REQUEST was because there was a security requirement for authenticated sessions to expire quickly (a matter of minutes), but that expiry time needed to be updated on each request (so the expiry would only occur after inactivity, not in the middle of usage). Currently SESSION_SAVE_EVERY_REQUEST is the only way to get Django to update the session cookie expiry time on each request.

That use case didn't have any need for saving empty sessions for unauthenticated users, though.

[Damon Timm]

So I used it in django-hitcount as one of the methods to identify a unique visitor as part of the view logic -- it was just the low hanging fruit at the time. Smile. If that setting is going away it may just be time to look into how to do it a little better.

​https://github.com/thornomad/django-hitcount

[Carl Meyer]

The setting isn't going away, but the behavior of creating empty session records for all visitors isn't coming back, I don't think.

Probably worth a mention in the docs for SESSION_SAVE_EVERY_REQUEST, though.

[Tim Graham]

​PR

[Tim Graham <timograham@…>]

In abf5ccc2:

Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't create empty sessions.

[Tim Graham <timograham@…>]

In adc9fa83:

[1.9.x] Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't create empty sessions.

Backport of abf5ccc29c45d53ec17541179bb5f0a75b28915d from master

[Tim Graham <timograham@…>]

In 12f4db23:

[1.8.x] Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't create empty sessions.

Backport of abf5ccc29c45d53ec17541179bb5f0a75b28915d from master