Code

Ticket #7515: clear_and_destroy2.diff

File clear_and_destroy2.diff, 8.4 KB (added by mrts, 6 years ago)

Remove redundant .save()s from tests and update to r8158

Line 
1Index: django/contrib/sessions/middleware.py
2===================================================================
3--- django/contrib/sessions/middleware.py       (revision 8160)
4+++ django/contrib/sessions/middleware.py       (working copy)
5@@ -16,12 +16,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@@ -30,7 +36,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 8160)
54+++ django/contrib/sessions/tests.py    (working copy)
55@@ -22,6 +22,28 @@
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.destroy()
65+>>> db_session.exists(db_session.session_key)
66+False
67+>>> db_session.exists(prev_key)
68+False
69+>>> db_session.session_key == prev_key
70+False
71+>>> len(db_session.session_key)
72+32
73+>>> db_session.modified, db_session.accessed
74+(False, True)
75+>>> db_session['foo'] = 'bar'
76+>>> db_session.modified, db_session.accessed
77+(True, True)
78+>>> db_session.save()
79+>>> db_session.exists(db_session.session_key)
80+True
81 
82 >>> file_session = FileSession()
83 >>> file_session.modified
84@@ -39,6 +61,28 @@
85 >>> file_session.delete(file_session.session_key)
86 >>> file_session.exists(file_session.session_key)
87 False
88+>>> file_session['foo'] = 'bar'
89+>>> file_session.save()
90+>>> file_session.exists(file_session.session_key)
91+True
92+>>> prev_key = file_session.session_key
93+>>> file_session.destroy()
94+>>> file_session.exists(file_session.session_key)
95+False
96+>>> file_session.exists(prev_key)
97+False
98+>>> file_session.session_key == prev_key
99+False
100+>>> len(file_session.session_key)
101+32
102+>>> file_session.modified, file_session.accessed
103+(False, True)
104+>>> file_session['foo'] = 'bar'
105+>>> file_session.modified, file_session.accessed
106+(True, True)
107+>>> file_session.save()
108+>>> file_session.exists(file_session.session_key)
109+True
110 
111 # Make sure the file backend checks for a good storage dir
112 >>> settings.SESSION_FILE_PATH = "/if/this/directory/exists/you/have/a/weird/computer"
113@@ -61,6 +105,28 @@
114 >>> cache_session.delete(cache_session.session_key)
115 >>> cache_session.exists(cache_session.session_key)
116 False
117+>>> cache_session['foo'] = 'bar'
118+>>> cache_session.save()
119+>>> cache_session.exists(cache_session.session_key)
120+True
121+>>> prev_key = cache_session.session_key
122+>>> cache_session.destroy()
123+>>> cache_session.exists(cache_session.session_key)
124+False
125+>>> cache_session.exists(prev_key)
126+False
127+>>> cache_session.session_key == prev_key
128+False
129+>>> len(cache_session.session_key)
130+32
131+>>> cache_session.modified, cache_session.accessed
132+(False, True)
133+>>> cache_session['foo'] = 'bar'
134+>>> cache_session.modified, cache_session.accessed
135+(True, True)
136+>>> cache_session.save()
137+>>> cache_session.exists(cache_session.session_key)
138+True
139 
140 >>> s = SessionBase()
141 >>> s._session['some key'] = 'exists' # Pre-populate the session with some data
142@@ -147,8 +213,17 @@
143 >>> list(i)
144 [('x', 1)]
145 
146-
147+# test .clear()
148+>>> s.modified = s.accessed = False
149+>>> s.items()
150+[('x', 1)]
151+>>> s.clear()
152+>>> s.items()
153+[]
154+>>> s.accessed, s.modified
155+(True, True)
156 
157+
158 #########################
159 # Custom session expiry #
160 #########################
161Index: django/contrib/sessions/backends/base.py
162===================================================================
163--- django/contrib/sessions/backends/base.py    (revision 8160)
164+++ django/contrib/sessions/backends/base.py    (working copy)
165@@ -25,6 +25,7 @@
166         self._session_key = session_key
167         self.accessed = False
168         self.modified = False
169+        self.destroyed = False
170 
171     def __contains__(self, key):
172         return key in self._session
173@@ -53,6 +54,23 @@
174         self.modified = self.modified or key in self._session
175         return self._session.pop(key, *args)
176 
177+    def clear(self):
178+        self._session.clear()
179+        self.modified = True
180+
181+    def destroy(self):
182+        self._session.clear()
183+        try:
184+            # remove old session, may fail
185+            self.delete(self._session_key)
186+        except:
187+            # ignore exceptions to guarantee session key reset
188+            pass
189+        self._session_key = self._get_new_session_key(safe=True)
190+        self.modified = False
191+        self.accessed = True
192+        self.destroyed = True
193+
194     def setdefault(self, key, value):
195         if key in self._session:
196             return self._session[key]
197@@ -107,7 +125,7 @@
198     def iteritems(self):
199         return self._session.iteritems()
200 
201-    def _get_new_session_key(self):
202+    def _get_new_session_key(self, safe=False):
203         "Returns session key that isn't being used."
204         # The random module is seeded when this Apache child is created.
205         # Use settings.SECRET_KEY as added salt.
206@@ -119,8 +137,17 @@
207         while 1:
208             session_key = md5.new("%s%s%s%s" % (random.randint(0, sys.maxint - 1),
209                                   pid, time.time(), settings.SECRET_KEY)).hexdigest()
210-            if not self.exists(session_key):
211-                break
212+            if safe:
213+                try:
214+                    if not self.exists(session_key):
215+                        break
216+                except:
217+                    # Depends on the assumption that key collisions are rare,
218+                    # see ticket #1180
219+                    pass
220+            else:
221+                if not self.exists(session_key):
222+                    break
223         return session_key
224 
225     def _get_session_key(self):
226Index: docs/sessions.txt
227===================================================================
228--- docs/sessions.txt   (revision 8160)
229+++ docs/sessions.txt   (working copy)
230@@ -106,8 +106,18 @@
231 
232     * ``setdefault()`` (**New in Django development version**)
233 
234+    * ``clear()`` (**New in Django development version**)
235+
236 It also has these methods:
237 
238+    * ``destroy()``
239+
240+      **New in Django development version**
241+
242+      Clears session data, deletes the old session from session storage and
243+      creates a new empty session in exception-safe manner. This is the
244+      recommended, safe way of resetting sessions on e.g. user logout.
245+
246     * ``set_test_cookie()``
247 
248       Sets a test cookie to determine whether the user's browser supports