[Django] #25489: SESSION_SAVE_EVERY_REQUEST Appears to have broke in 1.8.4
Django
----------------------------------+--------------------
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25489>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:1>
Django
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thornomad):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:2>
Django
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:3>
Django
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by carljm):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:4>
Django
Reporter: thornomad | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.sessions | Version: 1.8
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by thornomad):
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]
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:5>
Django
Reporter: thornomad | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* component: contrib.sessions => Documentation
* type: Uncategorized => Cleanup/optimization
* stage: Unreviewed => Accepted
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:6>
Django
Reporter: thornomad | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/5507 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:7>
Django
Reporter: thornomad | Owner: nobody
Type: | Status: new
Cleanup/optimization |
Component: Documentation | Version: 1.8
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by carljm):
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:8>
Django
Reporter: thornomad | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 1.8
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: new => closed
* resolution: => fixed
Comment:
In [changeset:"abf5ccc29c45d53ec17541179bb5f0a75b28915d" abf5ccc2]:
{{{
#!CommitTicketReference repository=""
revision="abf5ccc29c45d53ec17541179bb5f0a75b28915d"
Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't create
empty sessions.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:9>
Django
Reporter: thornomad | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"adc9fa8324f087e822653894bd0b97df8601b5cd" adc9fa83]:
{{{
#!CommitTicketReference repository=""
revision="adc9fa8324f087e822653894bd0b97df8601b5cd"
[1.9.x] Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't
create empty sessions.
Backport of abf5ccc29c45d53ec17541179bb5f0a75b28915d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:10>
Django
Reporter: thornomad | Owner: nobody
Type: | Status: closed
Cleanup/optimization |
Component: Documentation | Version: 1.8
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"12f4db23aaabc99e68c13198c8813888da079282" 12f4db23]:
{{{
#!CommitTicketReference repository=""
revision="12f4db23aaabc99e68c13198c8813888da079282"
[1.8.x] Fixed #25489 -- Documented that SESSION_SAVE_EVERY_REQUEST doesn't
create empty sessions.
Backport of abf5ccc29c45d53ec17541179bb5f0a75b28915d from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/25489#comment:11>