Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#22938 closed Bug (fixed)

clearsessions doesn't remove file-based sessions

Reported by: atarkowska@… Owned by: nobody
Component: contrib.sessions Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

My Django app uses file based session engine. I am currently experience issues with cleaning session files from tmp directory. Basically running clearsessions doesn't remove them at all.

I have been playing a bit with that to try various configurations and debugging Django code. Basically, get_expiry_age

https://github.com/django/django/blob/stable/1.6.x/django/contrib/sessions/backends/file.py#L90 returns cookie age rather then negative value.

That seems to be due to session_data.get('_session_expiry') being None. Is that a bug in file based session backend?

Change History (15)

comment:1 by anonymous, 10 years ago

My settings:

SESSION_ENGINE = 'django.contrib.sessions.backends.file'
SESSION_FILE_PATH = tempfile.gettempdir()
SESSION_COOKIE_AGE = 10 # 1 day in sec (86400)

comment:2 by Tim Graham, 10 years ago

Triage Stage: UnreviewedAccepted

I think removing expiry=session_data.get('_session_expiry') from the get_expiry_age() call you linked above would fix it. Could you test that? If it works and you could submit a patch with a regression test, that would be great.

comment:3 by anonymous, 10 years ago

I am afraid removing expiry=session_data.get('_session_expiry') as mentioned above throws an exception

patch https://github.com/aleksandra-tarkowska/django/commit/e972ea8be732462628faa877627260545bb8661c

Exception Type: 	RuntimeError
Exception Value: 	maximum recursion depth exceeded in cmp

comment:4 by anonymous, 10 years ago

  File "/omero/dist/lib/python/django/core/handlers/base.py", line 201, in get_response
    response = middleware_method(request, response)

  File "/omero/dist/lib/python/django/contrib/sessions/middleware.py", line 28, in process_response
    if request.session.get_expire_at_browser_close():

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 258, in get_expire_at_browser_close
    if self.get('_session_expiry') is None:

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 58, in get
    return self._session.get(key, default)

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 173, in _get_session
    self._session_cache = self.load()

  File "/omero/dist/lib/python/django/contrib/sessions/backends/file.py", line 91, in load
    modification=self._last_modification())

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 194, in get_expiry_age
    expiry = self.get('_session_expiry')

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 58, in get
    return self._session.get(key, default)

  File "/omero/dist/lib/python/django/contrib/sessions/backends/base.py", line 173, in _get_session
  ... looping 

comment:5 by atarkowska@…, 10 years ago

Can I ask what exactly set expire_date in file based session backend?

comment:6 by Tim Graham, 10 years ago

It looks like clearsessions only works for file-based sessions that have had set_expiry() called on them (nothing within Django itself calls this method). See #18194 for the ticket where this was implemented. It may be better to work address the problem via #19201 than to try to solve this ticket piecemeal for the file backend. Note that get_expiry_age() is working as documented.

comment:7 by atarkowska@…, 10 years ago

Basically request.session.get_expiry_date() is set but not propagated to _session_expiry
It is true that only calling

request.session.set_expiry(datetime.timedelta(seconds=settings.SESSION_COOKIE_AGE))                 

does work but clearsession still not remove expired files

delta 0:00:28.355096
sessionidhb39w1m0mehdpglp0esl2bry7gftd38n expiry_age: 28 _last_modification: 2014-07-02 16:20:28 _session_expiry: 2014-07-02 16:20:56.355096
delta 0:00:30.383044
sessionidynbrmr6cyjxgahkkc3omhbbg52s0axbt expiry_age: 30 _last_modification: 2014-07-02 16:23:18 _session_expiry: 2014-07-02 16:23:48.383044

This will never happen because delta will always be positive https://github.com/django/django/blob/stable/1.6.x/django/contrib/sessions/backends/base.py#L200

comment:8 by atarkowska@…, 10 years ago

The other thing I noticed that expory_age returned by get_expiry_age is changing. If call more requests containing request.session.modified = True this value is going down but the best I could achieve is 3 sec

sessionid44iuant7vkklpqqak09paaprh6wngjve expiry_age: 3 _last_modification: 2014-07-02 16:29:02 _session_expiry: 2014-07-02 16:29:05.224180

comment:9 by atarkowska@…, 10 years ago

This commit should resolve the issue. Basically the problem was that delta = expiry - modification was always positive because modification time was always before expiry. Now modification is set to the current time.

comment:10 by atarkowska@…, 10 years ago

My previous patch only works if _session_expiry is set. Perhaps is better idea to add function to test expiry date. That final changes I will be adapting to my Django app are here

comment:11 by Tim Graham, 10 years ago

Has patch: set
Needs tests: set

PR, but lacking a test.

comment:12 by Tim Graham, 10 years ago

Needs tests: unset

comment:13 by Tim Graham, 9 years ago

Patch needs improvement: set
Summary: clearsessions not remore session files from tmpclearsessions doesn't remove file-based sessions

comment:14 by Tim Graham <timograham@…>, 9 years ago

Resolution: fixed
Status: newclosed

In c0552247:

Fixed #22938 -- Allowed clearsessions to remove file-based sessions.

comment:15 by Tim Graham <timograham@…>, 9 years ago

In a3fffdc:

Fixed #25558 -- Fixed nondeterministic test failure on Windows: test_clearsessions_command.

The test session without an expiration date added in refs #22938 wasn't
always deleted on Windows because get_expiry_age() returns zero and the
file backend didn't consider that an expired session.

Note: See TracTickets for help on using tickets.
Back to Top