Opened 8 years ago

Closed 8 years ago

#26764 closed Cleanup/optimization (fixed)

Session.cycle_key() throws Exception if the session hasn't been accessed

Reported by: adam-iris Owned by: Adam Zapletal
Component: contrib.sessions Version: 1.8
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

A common use case I see is that a user changes some content then tries to view their changes on the website, but they get an older, previously-cached page.

Since the page-level cache key uses the session id, my solution is to call request.session.cycle_key(). To the user this is presented as a "hard reload" -- it clears the session key then redirects back to the current page, the page-level cache will miss and the user will get a newly-generated version.

However, this raised an exception:
'SessionStore' object has no attribute '_session_cache'

Upon inspection, it looks like SessionBase._session_cache doesn't get initialized except by calling SessionBase._session. But the cycle_key() method accesses _session_cache without ensuring that it's been initialized.

My workaround was to prepend a meaningless access call:

request.session.get('foo')
request.session.cycle_key()

This fixed the exception, but I think the proper fix is for cycle_key() to initialize (or more carefully check) _session_cache.

Change History (7)

comment:1 by Tim Graham, 8 years ago

Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

comment:2 by Adam Zapletal, 8 years ago

Owner: changed from nobody to Adam Zapletal
Status: newassigned

comment:3 by Adam Zapletal, 8 years ago

PR is here: https://github.com/django/django/pull/7031

This is a simple fix, but I'm wondering why _session_cache isn't set to {} in __init__. Maybe a discussion for another ticket.

Thanks!

comment:4 by Tim Graham, 8 years ago

Did you try making that change and seeing if any tests fail? Might be cleaner than the proposed try/except.

comment:5 by Adam Zapletal, 8 years ago

Yeah, with self._session_cache = {} in __init__, the sessions_tests are OK, but instead of passing with two skips and an expected failure, they pass with one unexpected success.

However, the full Django test suite does not pass with this change. It had 511 failures and 55 errors...so something deeper is going on here. Unexpected! Or maybe I'm missing something.

I'd like to look into it more and understand contrib.sessions better in general as a separate effort.

Last edited 8 years ago by Adam Zapletal (previous) (diff)

comment:6 by Adam Zapletal, 8 years ago

Has patch: set

comment:7 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 887f3d32:

Fixed #26764 -- Fixed Session.cycle_key() crash on unaccessed session.

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