Code

Ticket #7515: clear_and_destroy.diff

File clear_and_destroy.diff, 8.5 KB (added by mrts, 6 years ago)

Exception-safe destroy() added to request.session, with tests and docs

Line 
1Index: django/contrib/sessions/middleware.py
2===================================================================
3--- django/contrib/sessions/middleware.py       (revision 8137)
4+++ django/contrib/sessions/middleware.py       (working copy)
5@@ -21,12 +21,18 @@
6         try:
7             accessed = request.session.accessed
8             modified = request.session.modified
9+            destroyed = request.session.destroyed
10         except AttributeError:
11+            # request doesn't have 'session' attribute
12+            # FIXME: is it such a good idea to hide this exception?
13+            # if process_request() fails to set request.session we should
14+            # raise here as well instead of silently failing,
15+            # perhaps a SessionNotSetError?
16             pass
17         else:
18             if accessed:
19                 patch_vary_headers(response, ('Cookie',))
20-            if modified or settings.SESSION_SAVE_EVERY_REQUEST:
21+            if modified or destroyed or settings.SESSION_SAVE_EVERY_REQUEST:
22                 if request.session.get_expire_at_browser_close():
23                     max_age = None
24                     expires = None
25@@ -35,7 +41,24 @@
26                     expires_time = time.time() + max_age
27                     expires = cookie_date(expires_time)
28                 # Save the session data and refresh the client cookie.
29-                request.session.save()
30+                needs_saving = modified or settings.SESSION_SAVE_EVERY_REQUEST
31+                if destroyed and not needs_saving:
32+                    # New session not modified after destruction.
33+                    # Note that not saving here results in new key generation
34+                    # in load() on the next request. If this is undesirable,
35+                    # the three-branched needs_saving logic is not required.
36+                    pass
37+                elif destroyed and needs_saving:
38+                    # New session modified after destruction.  We
39+                    # need to guarantee no exception from the persistance
40+                    # layer can block sending the new session cookie to
41+                    # client.
42+                    try:
43+                        request.session.save()
44+                    except:
45+                        pass
46+                else:
47+                    request.session.save()
48                 response.set_cookie(settings.SESSION_COOKIE_NAME,
49                         request.session.session_key, max_age=max_age,
50                         expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
51Index: django/contrib/sessions/tests.py
52===================================================================
53--- django/contrib/sessions/tests.py    (revision 8137)
54+++ django/contrib/sessions/tests.py    (working copy)
55@@ -22,6 +22,29 @@
56 >>> db_session.delete(db_session.session_key)
57 >>> db_session.exists(db_session.session_key)
58 False
59+>>> db_session['foo'] = 'bar'
60+>>> db_session.save()
61+>>> db_session.exists(db_session.session_key)
62+True
63+>>> prev_key = db_session.session_key
64+>>> db_session.save()
65+>>> db_session.destroy()
66+>>> db_session.exists(db_session.session_key)
67+False
68+>>> db_session.exists(prev_key)
69+False
70+>>> db_session.session_key == prev_key
71+False
72+>>> len(db_session.session_key)
73+32
74+>>> db_session.modified, db_session.accessed
75+(False, True)
76+>>> db_session['foo'] = 'bar'
77+>>> db_session.modified, db_session.accessed
78+(True, True)
79+>>> db_session.save()
80+>>> db_session.exists(db_session.session_key)
81+True
82 
83 >>> file_session = FileSession()
84 >>> file_session.modified
85@@ -39,6 +62,29 @@
86 >>> file_session.delete(file_session.session_key)
87 >>> file_session.exists(file_session.session_key)
88 False
89+>>> file_session['foo'] = 'bar'
90+>>> file_session.save()
91+>>> file_session.exists(file_session.session_key)
92+True
93+>>> prev_key = file_session.session_key
94+>>> file_session.save()
95+>>> file_session.destroy()
96+>>> file_session.exists(file_session.session_key)
97+False
98+>>> file_session.exists(prev_key)
99+False
100+>>> file_session.session_key == prev_key
101+False
102+>>> len(file_session.session_key)
103+32
104+>>> file_session.modified, file_session.accessed
105+(False, True)
106+>>> file_session['foo'] = 'bar'
107+>>> file_session.modified, file_session.accessed
108+(True, True)
109+>>> file_session.save()
110+>>> file_session.exists(file_session.session_key)
111+True
112 
113 # Make sure the file backend checks for a good storage dir
114 >>> settings.SESSION_FILE_PATH = "/if/this/directory/exists/you/have/a/weird/computer"
115@@ -61,6 +107,29 @@
116 >>> cache_session.delete(cache_session.session_key)
117 >>> cache_session.exists(cache_session.session_key)
118 False
119+>>> cache_session['foo'] = 'bar'
120+>>> cache_session.save()
121+>>> cache_session.exists(cache_session.session_key)
122+True
123+>>> prev_key = cache_session.session_key
124+>>> cache_session.save()
125+>>> cache_session.destroy()
126+>>> cache_session.exists(cache_session.session_key)
127+False
128+>>> cache_session.exists(prev_key)
129+False
130+>>> cache_session.session_key == prev_key
131+False
132+>>> len(cache_session.session_key)
133+32
134+>>> cache_session.modified, cache_session.accessed
135+(False, True)
136+>>> cache_session['foo'] = 'bar'
137+>>> cache_session.modified, cache_session.accessed
138+(True, True)
139+>>> cache_session.save()
140+>>> cache_session.exists(cache_session.session_key)
141+True
142 
143 >>> s = SessionBase()
144 >>> s._session['some key'] = 'exists' # Pre-populate the session with some data
145@@ -147,8 +216,17 @@
146 >>> list(i)
147 [('x', 1)]
148 
149-
150+# test .clear()
151+>>> s.modified = s.accessed = False
152+>>> s.items()
153+[('x', 1)]
154+>>> s.clear()
155+>>> s.items()
156+[]
157+>>> s.accessed, s.modified
158+(True, True)
159 
160+
161 #########################
162 # Custom session expiry #
163 #########################
164Index: django/contrib/sessions/backends/base.py
165===================================================================
166--- django/contrib/sessions/backends/base.py    (revision 8137)
167+++ django/contrib/sessions/backends/base.py    (working copy)
168@@ -25,6 +25,7 @@
169         self._session_key = session_key
170         self.accessed = False
171         self.modified = False
172+        self.destroyed = False
173 
174     def __contains__(self, key):
175         return key in self._session
176@@ -53,6 +54,23 @@
177         self.modified = self.modified or key in self._session
178         return self._session.pop(key, *args)
179 
180+    def clear(self):
181+        self._session.clear()
182+        self.modified = True
183+
184+    def destroy(self):
185+        self._session.clear()
186+        try:
187+            # remove old session, may fail
188+            self.delete(self._session_key)
189+        except:
190+            # ignore exceptions to guarantee session key reset
191+            pass
192+        self._session_key = self._get_new_session_key(safe=True)
193+        self.modified = False
194+        self.accessed = True
195+        self.destroyed = True
196+
197     def setdefault(self, key, value):
198         if key in self._session:
199             return self._session[key]
200@@ -107,7 +125,7 @@
201     def iteritems(self):
202         return self._session.iteritems()
203 
204-    def _get_new_session_key(self):
205+    def _get_new_session_key(self, safe=False):
206         "Returns session key that isn't being used."
207         # The random module is seeded when this Apache child is created.
208         # Use settings.SECRET_KEY as added salt.
209@@ -119,8 +137,17 @@
210         while 1:
211             session_key = md5.new("%s%s%s%s" % (random.randint(0, sys.maxint - 1),
212                                   pid, time.time(), settings.SECRET_KEY)).hexdigest()
213-            if not self.exists(session_key):
214-                break
215+            if safe:
216+                try:
217+                    if not self.exists(session_key):
218+                        break
219+                except:
220+                    # Depends on the assumption that key collisions are rare,
221+                    # see ticket #1180
222+                    pass
223+            else:
224+                if not self.exists(session_key):
225+                    break
226         return session_key
227 
228     def _get_session_key(self):
229Index: docs/sessions.txt
230===================================================================
231--- docs/sessions.txt   (revision 8137)
232+++ docs/sessions.txt   (working copy)
233@@ -106,8 +106,18 @@
234 
235     * ``setdefault()`` (**New in Django development version**)
236 
237+    * ``clear()`` (**New in Django development version**)
238+
239 It also has these methods:
240 
241+    * ``destroy()``
242+
243+      **New in Django development version**
244+
245+      Clears session data, deletes the old session from session storage and
246+      creates a new empty session in exception-safe manner. This is the
247+      recommended, safe way of resetting sessions on e.g. user logout.
248+
249     * ``set_test_cookie()``
250 
251       Sets a test cookie to determine whether the user's browser supports