[Django] #21124: Default session data serializer doesn't support extended data types
Django
----------------------------------+------------------------
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6-beta-1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+------------------------
#20922 introduced the option to choose a custom session data serializer.
The default option is to use the new JSONSerializer starting from 1.6,
since using pickle would lead to a remote code execution vulnerability
when session data is stored in cookies.
While this can be considered a sensible security choice, it becomes
inconvenient as the JSON encoder used by JSONSerializer is not the same
used elsewhere in Django, as it only support basic data types: string,
int/floats, booleans, nested dicts and lists, None.
The inconvenience is breaking compatibility with all third party apps that
rely on storing extended data types (such as those supported by
DjangoJSONEncoder) with the default settings. Properly serializing
datetime (possibly tz-aware) can be hard, and changing the default puts
the burden on third party apps coders.
They would have the option to either add two complexity layers (properly
serializing/deserializing datetime objects, and not breaking compatibility
with the previous versions of the same app), or to break compatibility
with Django default settings.
As an example of commonly used data types that can't be stored anymore
with default settings:
- datetime, timedelta objects (supported by DjangoJSONEncoder)
- decimal objects (supported by DjangoJSONEncoder)
- arbitrary binary strings
- Geometry objects
I think the option of reverting the default to pickle should be also
considered.
--
Ticket URL: <https://code.djangoproject.com/ticket/21124>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.6-beta-1
Severity: Normal | Resolution: wontfix
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 timo):
* status: new => closed
* needs_better_patch: => 0
* resolution: => wontfix
* needs_tests: => 0
* needs_docs: => 0
Comment:
Thanks for your thoughts. I think most of the points you've raised were
discussed during the implementation of this, either on the ticket (#20922)
or on the linked [https://github.com/django/django/pull/1488 pull request]
(or the [https://docs.djangoproject.com/en/dev/topics/http/sessions
/#session-serialization documentation itself]). Could you please take a
look at the discussion there if you haven't? If after reading that you
still have disagreements, please raise the issue on django-developers
rather than this ticket tracker. Thanks!
Suggestions for documentations edits or additions would also be welcome.
p.s. To address one of your points, one of the decisions was indeed to put
the burden on third party app coders to serialize session data as simple
data types like strings which would be compatible with JSON. We made this
change to `contrib.messages` for example.
--
Ticket URL: <https://code.djangoproject.com/ticket/21124#comment:1>
Django
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by victor@…):
* status: closed => new
* needs_docs: 0 => 1
* resolution: wontfix =>
* needs_tests: 0 => 1
Comment:
Hey, I also ran into this issue that broke quite huge apps such as django-
allauth and its entire oauth2 feature. Took me hours and hours to find out
that the session cache was not serializing well - there was no clue
whatsoever.
In a nutshell, deep down django-allauth puts objects such as ['foo'] (a
list of one string) in the session cache; when it's read again, it's been
transformed to 'foo' (no more list). No exception, or warning of any sort
- I guess that's the real issue, right ?
I'm not sure about how to cope with this, but there are several ways:
maybe communicate more on this change, add some tests (I place X in
session data, I retrieve X, not Y) and probably exceptions if trying to
store something unsupported.
--
Ticket URL: <https://code.djangoproject.com/ticket/21124#comment:2>
Django
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by victor@…):
Sorry for the formatting above, the object put in the session cache reads
{{{
['foo']
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/21124#comment:3>
Django
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6-beta-1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Comment (by timo):
I'm unable to reproduce the list to non-list transformation you are
describing. For example, I modified
`django.contrib.sessions.tests.DatabaseSessionTests.test_session_get_decoded`
to store a list (`['1']`) and `get_decoded()` return a list as expected.
Could you include a sample shell session or a test for Django's test suite
that demonstrates the behavior?
--
Ticket URL: <https://code.djangoproject.com/ticket/21124#comment:4>
Django
Reporter: sorcio@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.6-beta-1
Severity: Normal | Resolution: wontfix
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timo):
* status: new => closed
* resolution: => wontfix
Comment:
Please open a new ticket if there is indeed a bug here.
--
Ticket URL: <https://code.djangoproject.com/ticket/21124#comment:5>