#18458 closed Bug (fixed)
Method session.get_expiry_age() does not return what the doc says
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | contrib.sessions | Version: | 1.4 |
Severity: | Normal | Keywords: | session expiration expiry age get_expiry_age |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
This is either a bug in the doc or in the code.
The doc for the SessionBase.get_expiry_age() method [1] says:
get_expiry_age()
Returns the number of seconds until this session expires. For sessions with no custom expiration (or those set to expire at browser close), this will equal SESSION_COOKIE_AGE.
This means that it should return the session's remaining time to live when expiration has been customized using one of these:
A) set_expiry(seconds) # integer != 0
B) set_expiry(datetime)
C) set_expiry(timedelta)
It works as documented in the cases B and C. But when set_expiry() has been used with seconds (A), then get_expiry_age() does the unexpected thing of returning the exact value that has been passed to set_expiry(). In other words, when you give 300 to set_expiry(), get_expiry_age() will always return 300. That is not “the number of seconds until this session expires”.
Looking at the code [2], it is obvious that a difference is calculated only when the expiry value is a datetime:
def get_expiry_age(self): """Get the number of seconds until the session expires.""" expiry = self.get('_session_expiry') if not expiry: # Checks both None and 0 cases return settings.SESSION_COOKIE_AGE if not isinstance(expiry, datetime): return expiry delta = expiry - timezone.now() return delta.days * 86400 + delta.seconds
I'm not sure about the purpose of this method and I don't know the design decisions that led to it, but I assume that if you don't want this function to use the session's last-modified datetime, then you may just need to edit the documentation to clarify this point (and maybe the docstring).
On the other hand, in my humble opinion, I think that this method’s behavior is quite surprising (even if it were working as documented) because the value it returns really depends on how the expiration has been defined. I believe it would be nice to make it return the number of seconds until the session expires for any active session, with no exceptional cases. And while we're at it, maybe also rename it to something that reflect better what it does, for instance "get_remaining_time_to_live" or "get_time_left". (Then again, maybe I didn't understand the purpose of this method.)
I hope this is helping.
Anyway, thank you all for this great framework!
Martin Bouladour
[1] https://docs.djangoproject.com/en/1.4/topics/http/sessions/#django.contrib.sessions.backends.base.SessionBase.get_expiry_age
[2] https://github.com/django/django/blob/1.4/django/contrib/sessions/backends/base.py#L170
Change History (4)
comment:1 by , 12 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|
comment:2 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:3 by , 12 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
I recently worked on this code and came to the conclusion that
get_expiry_age
andget_expiry_date
are designed to work at the time when the session is saved, and not at any other point in time.get_expiry_age
is used to determine the session cookie's maximum age. That probably explains the name. Since it's a public API, it's better to keep the current name, even if it isn't great.In order to fix #18194, I may have to introduced an optional parameter to pass the "reference time" (ie. the time the session was saved at); if not provided, this parameter would default to the current time. That would resolve this issue.
Otherwise, it'll just be a documentation issue.