[Django] #23764: sessions.backends.cache.SessionStore does not respect settings.SESSION_SERIALIZER
Django
settings.SESSION_SERIALIZER
--------------------------------------+----------------------------------
Reporter: coagulant | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 1.7
Severity: Normal | Keywords: pickle json sessions
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+----------------------------------
According to [https://docs.djangoproject.com/en/1.6/topics/http/sessions
/#session-serialization Session serialization docs], `SESSION_SERIALIZER`
setting, introduced in 1.5.3, allows to customize how session data is
stored. `JSONSerializer` is used as default since 1.6.
All session backends honor this setting, except for
`sessions.backends.cache.SessionStore`, which still uses pickle as
serialization format (not directly, but via cache backend).
It should be documented that cache store ignores this setting or
SessionStore should be modified to support it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23764>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 1.7
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by manfre):
* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* stage: Unreviewed => Accepted
Comment:
The cache session backend should be consistent with the other session
backends.
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:1>
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: contrib.sessions | Version: 1.7
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by r4vi):
Looks like this is still the case as of django 1.8rc; I am happy to fix by
making `sessions.backends.cache.SessionStore` respect the
`SESSION_SERIALIZER` setting if you think that's the right approach or I
can fix the documentation to show that
`sessions.backends.cache.SessionStore` doesn't support it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:2>
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: r4vi
Type: Cleanup/optimization | Status: assigned
Component: contrib.sessions | Version: 1.7
Severity: Normal | Resolution:
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by r4vi):
* status: new => assigned
* owner: nobody => r4vi
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:3>
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: r4vi
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by erikr):
I would agree with manfre that this should be consistent, and therefore be
fixed. However, it would be worth looking into the commit and ticket that
added this setting and modified the other backends, to see whether this
was perhaps intentional for some special reason.
This would be a backwards incompatible change as well: if users are
running sessions with the cache backend and have not explicitly set
`SESSION_SERIALIZER` to `PickleSerializer`, upgrading will cause all
sessions to be invalidated. (The default value of that setting is
`JSONSerializer`.) Despite that, I still think this change is worth
making, though it will have to be included in the release notes for 1.9.
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:4>
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: r4vi
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by r4vi):
I've created a PR: https://github.com/django/django/pull/4372 and I'll
repeat the commnt here:
Have fixed `cache.SessionStore` to respect the SESSION_SERIALIZER setting.
The `django.contrib.sessions.backends.cache` now `.encode`s in the .save
method and `.decode`s in the load method.
I'm not sure how to add regression tests for this. One option is to add
this to the test.session_tests.tests.SessionTestsMixin:
{{{
def test_save_encodes(self):
# regression test for #23764
session = self.backend()
session.encode = mock.MagicMock(return_value='encoded')
session['some key'] = 'some unencoded value'
session.save()
session.encode.assert_called_with({'some key': 'some unencoded
value'})
}}}
but this tests for breaks `tests.sessions_tests.tests.CookieSessionTests`
because that backend gives self.serializer to the signer instead of using
it directly. I don't want to put an ugly
{{{
def test_save_encdoes(self):
pass
}}}
in there.
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:5>
Django
settings.SESSION_SERIALIZER
Reporter: coagulant | Owner: r4vi
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: pickle json sessions | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by r4vi):
* needs_docs: 0 => 1
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23764#comment:6>