[1.7.x] Fixed #19905 -- Fixed leakage of file descriptors in form wizard.

Backport of c4c2c99669 from master.

django/contrib/formtools/tests/wizard/namedwizardtests/tests.py

@@ -1,5 +1,7 @@
 from __future__ import unicode_literals
 
+import copy
+
 from django.core.urlresolvers import reverse
 from django.http import QueryDict
 from django.test import TestCase
@@ -11,14 +13,31 @@ from django.contrib.formtools.wizard.views import (NamedUrlSessionWizardView,
                                                    NamedUrlCookieWizardView)
 from django.contrib.formtools.tests.wizard.test_forms import get_request, Step1, Step2
 
+from .forms import temp_storage
+
+
+UPLOADED_FILE_NAME = 'tests.py'
+
 
 class NamedWizardTests(object):
     urls = 'django.contrib.formtools.tests.wizard.namedwizardtests.urls'
 
     def setUp(self):
         self.testuser, created = User.objects.get_or_create(username='testuser1')
+        # Get new step data, since we modify it during the tests.
+        self.wizard_step_data = copy.deepcopy(self.wizard_step_data)
         self.wizard_step_data[0]['form1-user'] = self.testuser.pk
 
+    def tearDown(self):
+        # Ensure that there are no files in the storage which could lead to false
+        # results in the next tests. Deleting the whole storage dir is not really
+        # an option since the storage is defined on the module level and can't be
+        # easily reinitialized. (FIXME: The tests here should use the view classes
+        # directly instead of the test client, then the storage issues would go
+        # away too.)
+        for file in temp_storage.listdir('')[1]:
+            temp_storage.delete(file)
+
     def test_initial_call(self):
         response = self.client.get(reverse('%s_start' % self.wizard_urlname))
         self.assertEqual(response.status_code, 302)
@@ -123,17 +142,21 @@ class NamedWizardTests(object):
         self.assertEqual(response.context['wizard']['steps'].current, 'form2')
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'].close()
+        with open(__file__, 'rb') as post_file:
-        post_data['form2-file1'] = open(__file__, 'rb')
+            post_data['form2-file1'] = post_file
-        response = self.client.post(
+            response = self.client.post(
-            reverse(self.wizard_urlname,
+                reverse(self.wizard_urlname,
-                    kwargs={'step': response.context['wizard']['steps'].current}),
+                        kwargs={'step': response.context['wizard']['steps'].current}),
-            post_data)
+                post_data)
         response = self.client.get(response.url)
 
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context['wizard']['steps'].current, 'form3')
 
+        # Check that the file got uploaded properly.
+        with open(__file__, 'rb') as f, temp_storage.open(UPLOADED_FILE_NAME) as f2:
+            self.assertEqual(f.read(), f2.read())
+
         response = self.client.post(
             reverse(self.wizard_urlname,
                     kwargs={'step': response.context['wizard']['steps'].current}),
@@ -150,10 +173,10 @@ class NamedWizardTests(object):
         response = self.client.get(response.url)
         self.assertEqual(response.status_code, 200)
 
+        # After the wizard is done no files should exist anymore.
+        self.assertFalse(temp_storage.exists(UPLOADED_FILE_NAME))
+
         all_data = response.context['form_list']
-        with open(__file__, 'rb') as f:
-            self.assertEqual(all_data[1]['file1'].read(), f.read())
-        all_data[1]['file1'].close()
         del all_data[1]['file1']
         self.assertEqual(all_data, [
             {'name': 'Pony', 'thirsty': True, 'user': self.testuser},
@@ -174,22 +197,22 @@ class NamedWizardTests(object):
         self.assertEqual(response.status_code, 200)
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'] = open(__file__, 'rb')
+        with open(__file__, 'rb') as post_file:
-        response = self.client.post(
+            post_data['form2-file1'] = post_file
-            reverse(self.wizard_urlname,
+            response = self.client.post(
-                    kwargs={'step': response.context['wizard']['steps'].current}),
+                reverse(self.wizard_urlname,
-            post_data)
+                        kwargs={'step': response.context['wizard']['steps'].current}),
+                post_data)
         response = self.client.get(response.url)
         self.assertEqual(response.status_code, 200)
+        self.assertTrue(temp_storage.exists(UPLOADED_FILE_NAME))
 
         step2_url = reverse(self.wizard_urlname, kwargs={'step': 'form2'})
         response = self.client.get(step2_url)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context['wizard']['steps'].current, 'form2')
-        with open(__file__, 'rb') as f:
+        with open(__file__, 'rb') as f, temp_storage.open(UPLOADED_FILE_NAME) as f2:
-            self.assertEqual(
+            self.assertEqual(f.read(), f2.read())
-                response.context['wizard']['form'].files['form2-file1'].read(),
-                f.read())
 
         response = self.client.post(
             reverse(self.wizard_urlname,
@@ -206,9 +229,9 @@ class NamedWizardTests(object):
         self.assertEqual(response.status_code, 200)
 
         all_data = response.context['all_cleaned_data']
-        with open(__file__, 'rb') as f:
+        self.assertEqual(all_data['file1'].name, UPLOADED_FILE_NAME)
-            self.assertEqual(all_data['file1'].read(), f.read())
+        self.assertTrue(all_data['file1'].closed)
-        all_data['file1'].close()
+        self.assertFalse(temp_storage.exists(UPLOADED_FILE_NAME))
         del all_data['file1']
         self.assertEqual(
             all_data,
@@ -237,12 +260,12 @@ class NamedWizardTests(object):
         self.assertEqual(response.status_code, 200)
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'].close()
+        with open(__file__, 'rb') as post_file:
-        post_data['form2-file1'] = open(__file__, 'rb')
+            post_data['form2-file1'] = post_file
-        response = self.client.post(
+            response = self.client.post(
-            reverse(self.wizard_urlname,
+                reverse(self.wizard_urlname,
-                    kwargs={'step': response.context['wizard']['steps'].current}),
+                        kwargs={'step': response.context['wizard']['steps'].current}),
-            post_data)
+                post_data)
         response = self.client.get(response.url)
         self.assertEqual(response.status_code, 200)
 

django/contrib/formtools/tests/wizard/storage.py

@@ -85,3 +85,19 @@ class TestStorage(object):
         storage.extra_data['test'] = True
 
         self.assertTrue('test' in storage.extra_data)
+
+    def test_reset_deletes_tmp_files(self):
+        request = get_request()
+        storage = self.get_storage()('wizard1', request, temp_storage)
+
+        step = 'start'
+        file_ = SimpleUploadedFile('file.txt', b'content')
+        storage.set_step_files(step, {'file': file_})
+
+        with storage.get_step_files(step)['file'] as file:
+            tmp_name = file.name
+
+        self.assertTrue(storage.file_storage.exists(tmp_name))
+
+        storage.reset()
+        self.assertFalse(storage.file_storage.exists(tmp_name))

django/contrib/formtools/tests/wizard/wizardtests/tests.py

@@ -1,5 +1,6 @@
 from __future__ import unicode_literals
 
+import copy
 import os
 
 from django import forms
@@ -12,6 +13,11 @@ from django.contrib.formtools.wizard.views import CookieWizardView
 from django.utils._os import upath
 from django.contrib.formtools.tests.models import Poet, Poem
 
+from .forms import temp_storage
+
+
+UPLOADED_FILE_NAME = 'tests.py'
+
 
 class UserForm(forms.ModelForm):
     class Meta:
@@ -28,8 +34,20 @@ class WizardTests(object):
 
     def setUp(self):
         self.testuser, created = User.objects.get_or_create(username='testuser1')
+        # Get new step data, since we modify it during the tests.
+        self.wizard_step_data = copy.deepcopy(self.wizard_step_data)
         self.wizard_step_data[0]['form1-user'] = self.testuser.pk
 
+    def tearDown(self):
+        # Ensure that there are no files in the storage which could lead to false
+        # results in the next tests. Deleting the whole storage dir is not really
+        # an option since the storage is defined on the module level and can't be
+        # easily reinitialized. (FIXME: The tests here should use the view classes
+        # directly instead of the test client, then the storage issues would go
+        # away too.)
+        for file in temp_storage.listdir('')[1]:
+            temp_storage.delete(file)
+
     def test_initial_call(self):
         response = self.client.get(self.wizard_url)
         wizard = response.context['wizard']
@@ -98,11 +116,16 @@ class WizardTests(object):
         self.assertEqual(response.context['wizard']['steps'].current, 'form2')
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'] = open(upath(__file__), 'rb')
+        with open(upath(__file__), 'rb') as post_file:
-        response = self.client.post(self.wizard_url, post_data)
+            post_data['form2-file1'] = post_file
+            response = self.client.post(self.wizard_url, post_data)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context['wizard']['steps'].current, 'form3')
 
+        # Check that the file got uploaded properly.
+        with open(upath(__file__), 'rb') as f, temp_storage.open(UPLOADED_FILE_NAME) as f2:
+            self.assertEqual(f.read(), f2.read())
+
         response = self.client.post(self.wizard_url, self.wizard_step_data[2])
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context['wizard']['steps'].current, 'form4')
@@ -110,10 +133,10 @@ class WizardTests(object):
         response = self.client.post(self.wizard_url, self.wizard_step_data[3])
         self.assertEqual(response.status_code, 200)
 
+        # After the wizard is done no files should exist anymore.
+        self.assertFalse(temp_storage.exists(UPLOADED_FILE_NAME))
+
         all_data = response.context['form_list']
-        with open(upath(__file__), 'rb') as f:
-            self.assertEqual(all_data[1]['file1'].read(), f.read())
-        all_data[1]['file1'].close()
         del all_data[1]['file1']
         self.assertEqual(all_data, [
             {'name': 'Pony', 'thirsty': True, 'user': self.testuser},
@@ -134,6 +157,7 @@ class WizardTests(object):
             post_data['form2-file1'] = post_file
             response = self.client.post(self.wizard_url, post_data)
         self.assertEqual(response.status_code, 200)
+        self.assertTrue(temp_storage.exists(UPLOADED_FILE_NAME))
 
         response = self.client.post(self.wizard_url, self.wizard_step_data[2])
         self.assertEqual(response.status_code, 200)
@@ -142,9 +166,9 @@ class WizardTests(object):
         self.assertEqual(response.status_code, 200)
 
         all_data = response.context['all_cleaned_data']
-        with open(upath(__file__), 'rb') as f:
+        self.assertEqual(all_data['file1'].name, UPLOADED_FILE_NAME)
-            self.assertEqual(all_data['file1'].read(), f.read())
+        self.assertTrue(all_data['file1'].closed)
-        all_data['file1'].close()
+        self.assertFalse(temp_storage.exists(UPLOADED_FILE_NAME))
         del all_data['file1']
         self.assertEqual(all_data, {
             'name': 'Pony', 'thirsty': True, 'user': self.testuser,
@@ -161,9 +185,9 @@ class WizardTests(object):
         self.assertEqual(response.status_code, 200)
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'].close()
+        with open(upath(__file__), 'rb') as post_file:
-        post_data['form2-file1'] = open(upath(__file__), 'rb')
+            post_data['form2-file1'] = post_file
-        response = self.client.post(self.wizard_url, post_data)
+            response = self.client.post(self.wizard_url, post_data)
         self.assertEqual(response.status_code, 200)
 
         response = self.client.post(self.wizard_url, self.wizard_step_data[2])
@@ -189,9 +213,9 @@ class WizardTests(object):
         self.assertEqual(response.context['wizard']['steps'].current, 'form2')
 
         post_data = self.wizard_step_data[1]
-        post_data['form2-file1'].close()
+        with open(upath(__file__), 'rb') as post_file:
-        post_data['form2-file1'] = open(upath(__file__), 'rb')
+            post_data['form2-file1'] = post_file
-        response = self.client.post(self.wizard_url, post_data)
+            response = self.client.post(self.wizard_url, post_data)
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.context['wizard']['steps'].current, 'form3')
 

django/contrib/formtools/wizard/storage/base.py

@@ -16,6 +16,7 @@ class BaseStorage(object):
         self.prefix = 'wizard_%s' % prefix
         self.request = request
         self.file_storage = file_storage
+        self._files = {}
 
     def init_data(self):
         self.data = {
@@ -77,8 +78,10 @@ class BaseStorage(object):
         for field, field_dict in six.iteritems(wizard_files):
             field_dict = field_dict.copy()
             tmp_name = field_dict.pop('tmp_name')
-            files[field] = UploadedFile(
+            if (step, field) not in self._files:
-                file=self.file_storage.open(tmp_name), **field_dict)
+                self._files[(step, field)] = UploadedFile(
+                    file=self.file_storage.open(tmp_name), **field_dict)
+            files[field] = self._files[(step, field)]
         return files or None
 
     def set_step_files(self, step, files):
@@ -106,4 +109,12 @@ class BaseStorage(object):
         return self.get_step_files(self.current_step)
 
     def update_response(self, response):
-        pass
+        def post_render_callback(response):
+            for file in self._files.values():
+                if not file.closed:
+                    file.close()
+
+        if hasattr(response, 'render'):
+            response.add_post_render_callback(post_render_callback)
+        else:
+            post_render_callback(response)

django/contrib/formtools/wizard/storage/cookie.py

@@ -27,6 +27,7 @@ class CookieStorage(storage.BaseStorage):
         return json.loads(data, cls=json.JSONDecoder)
 
     def update_response(self, response):
+        super(CookieStorage, self).update_response(response)
         if self.data:
             response.set_signed_cookie(self.prefix, self.encoder.encode(self.data))
         else: