Session expiry dates should be in an ISO string instead of datetime

[n142857@…]

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.

[Tim Graham]

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 ​set_expiry().

[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.

[Aymeric Augustin]

I don't think the current behavior is the best we can do :)

[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.

[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.

[Mariusz Felisiak]

Fixed by 436862787cbdbd68b0ba20ed8c23b295e3679df3.