[Django] #21514: Session expiry dates should be in an ISO string instead of datetime
Django
----------------------------------+--------------------
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
----------------------------------+--------------------
The documentation for Django 1.6 says
(https://docs.djangoproject.com/en/dev/topics/http/sessions/):
Note that datetime and timedelta values are only serializable if you
are using the PickleSerializer.
And that's true. For instance, using django-social-auth I got this session
data:
{'facebook_state': 'zyuqNJsGiXBPubpnwV7Lgh5pqbkKiqk5',
'social_auth_last_login_backend': u'facebook', '_auth_user_id': 4,
'_auth_user_backend': 'social_auth.backends.facebook.FacebookBackend',
'_session_expiry': datetime.datetime(2013, 11, 18, 9, 21, 20, 75255,
tzinfo=<UTC>)}.
Which will give this error on login when using JSONSerializer:
datetime.datetime(2013, 11, 18, 9, 21, 20, 75255, tzinfo=<UTC>) is not
JSON serializable
Django ships now with JSONSerializer enabled, but Django is still actively
putting datetimes in sessions, concretely in
django/contrib/sessions/backends/base.py:251. Since it's not a Django
extension which is incompatible with the default behaviour, but Django
itself, I think it's Django which must be fixed.
A solution is to serialize the datetime to ISO 8601 before storing it in
the session.
--
Ticket URL: <https://code.djangoproject.com/ticket/21514>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
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):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Where is the code that calls `set_expiry()` with a datetime? My guess is
that it's somewhere in your own project or a third party module, not in
Django itself. See also the documentation for
[https://docs.djangoproject.com/en/dev/topics/http/sessions/#django.contrib.sessions.backends.base.SessionBase.set_expiry
set_expiry()].
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:1>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by anonymous):
You're right. it's django-social-auth (views.py, function
complete_process) who sets an expiry date. (For future readers who use
Django 1.6 and cannot log in through Facebook: use
`SOCIAL_AUTH_SESSION_EXPIRATION=False`).
But django-social-auth does the right thing in passing a datetime, because
it cannot do otherwise: passing a string to `set_expiry` is not documented
(it's also not as easy as passing a date). So I think set_expiry should
accept datetime (as it does now) and convert to string when storing.
At least, issue a warning/error in set_expiry if
`SESSION_SERIALIZER`=JsonSerializer.
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:2>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by aaugustin):
* stage: Unreviewed => Accepted
Comment:
I don't think the current behavior is the best we can do :)
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:3>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by prasoon2211):
Okay, the dates can of course be stored in the ISO format. But, there's a
problem.
{{{
s = Session.objects.get(
session_key=self.session_key,
expire_date__gt=timezone.now()
)
}}}
There are code segments like this in other files under
contrib/sessions/backends directory. Here, we are comparing between two
datetime objects. If we store expiry date as a string, I don't see how
will we be able to make such queries, at least now without using raw sql.
If there's a solution to this, then we can fix this issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:4>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by prasoon2211):
* cc: prasoon2211 (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:5>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: new
Component: contrib.sessions | Version: 1.6
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Matt Riviere):
If serializing/unserializing is handled within set_expiry() and
get_expiry_date/age(), the majority of user code should be unaffected. In
particular, what prasoon2211 mentioned would not be a problem, because the
db session adapter use get_expiry_date(), so it wouldn't even see the
change. session!['_expiry_date'] can already be either an int, a datetime,
or None, so adding an option to let it be an ISO8601 string doesn't seem
that bad.
The only thing that would break, is if someone is accessing
session!['_expiry_date'] directly (or parsing the stored session data from
outside django entirely) and expects to find a datetime. Nothing within
django does it. No idea how much outside code is doing it. It is probably
possible to keep backwards compatibility on that by serializing to ISO8601
only for non-pickle serializer but that seems to complicate things a bit
too much.
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:6>
Django
Reporter: n142857@… | Owner: nobody
Type: Bug | Status: closed
Component: contrib.sessions | Version: 1.6
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Mariusz Felisiak):
* status: new => closed
* resolution: => fixed
Comment:
Fixed by 436862787cbdbd68b0ba20ed8c23b295e3679df3.
--
Ticket URL: <https://code.djangoproject.com/ticket/21514#comment:7>