Ticket #680 (defect)
Opened 2 years ago
Last modified 1 year ago
sessions save() and close() assume init() was called
Status: closed (fixed)
| Reported by: | guest | Assigned to: | rdelon |
|---|---|---|---|
| Priority: | normal | Milestone: | 3.1 |
| Component: | CherryPy code | Keywords: | session |
| Cc: |
(I just started using CherryPy yesterday, so take this with a grain of salt)
If an exception (eg. HTTPError) is raised before sessions.init() is called, an exception will be raised when either sessions.save() or sessions.close() try to access cherrypy.session.
Example using http_methods_allowed() from http://tools.cherrypy.org/wiki/HTTPMethodFiltering:
- [on_start_resource] http_methods_allowed() decides the HTTP method is not allowed and raises a HTTPError.
- Note that [before_request_body] never happens and sessions.init() is not called.
- [before_finalize] sessions.save() is called and raises an exception when trying to access cherrypy.session. Boom!
Here is a patch against lib/sessions.py to make sure sessions.init() was called before accessing cherrypy.session:
--- sessions.py.orig 2007-03-30 10:59:59.000000000 +0100
+++ sessions.py 2007-03-30 11:40:01.000000000 +0100
@@ -395,6 +395,9 @@
def save():
"""Save any changed session data."""
+ # If an exception was raised, init may not have been called
+ if not hasattr(cherrypy.request, "_session_init_flag"):
+ return
# Guard against running twice
if hasattr(cherrypy.request, "_sessionsaved"):
return
@@ -414,10 +417,12 @@
def close():
"""Close the session object for this request."""
- sess = cherrypy.session
- if sess.locked:
- # If the session is still locked we release the lock
- sess.release_lock()
+ # If an exception was raised, init may not have been called
+ if hasattr(cherrypy.request, "_session_init_flag"):
+ sess = cherrypy.session
+ if sess.locked:
+ # If the session is still locked we release the lock
+ sess.release_lock()
close.failsafe = True
close.priority = 90
@@ -446,7 +451,8 @@
request = cherrypy.request
- # Guard against running twice
+ # Guard against running twice and signal save and close not
+ # to run if init was never called
if hasattr(request, "_session_init_flag"):
return
request._session_init_flag = True
Attachments
Change History
04/01/07 19:20:59: Modified by fumanchu
04/02/07 05:53:33: Modified by guest
- attachment cherrypy_session.patch added.
04/02/07 05:54:28: Modified by guest
Ah, I did not think of the genuine error, calling save() or close() without a prior call to init().
Here is an alternative solution which fixes the issue while preserving the error:
Do not hook save() and close() calls unconditionally in SessionTool._setup(). Instead, hook them in sessions.init(). So save() and close() are called iff init() was called. A call to those functions without a prior call to init() will continue raising an exception.
I hope this solution makes sense, see patch.
04/02/07 05:56:23: Modified by guest
To clarify: please ignore the original inline text patch and consider the attached patch file instead.
06/17/07 18:57:46: Modified by fumanchu
- status changed from new to closed.
- resolution set to fixed.
- milestone set to 3.1.
Fixed in [1673]. I went for a more data-driven approach, testing for hasattr(cherrypy, "session") instead of delaying the hooking. This should allow people to use the functions in sessions.py more easily even if they're not using the SessionTool?. It's important to keep Tool code separate from library code where possible.
08/22/07 12:10:49: Modified by fumanchu
Actual, working fix (and test!) in [1707]. ;)
I'm of mixed opinion on this: the reasoning given is fine, but part of me wants it to break so that "legitimate errors" don't pass silently.