Fixed #6209: handle `BooleanField`s in `FormPreview` and `FormWizard`. In the process, broke the the security hash calculation out to a helper function. Thanks to mcroydon and rajeshdhawan.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8597 bcc190cf-cafb-0310-a4f2-bffc1f526a37

django/contrib/formtools/preview.py

@@ -9,6 +9,7 @@ from django.http import Http404
 from django.shortcuts import render_to_response
 from django.template.context import RequestContext
 from django.utils.hashcompat import md5_constructor
+from django.contrib.formtools.utils import security_hash
 
 AUTO_ID = 'formtools_%s' # Each form here uses this as its auto_id parameter.
 
@@ -97,20 +98,12 @@ class FormPreview(object):
 
     def security_hash(self, request, form):
         """
-        Calculates the security hash for the given Form instance.
+        Calculates the security hash for the given HttpRequest and Form instances.
 
-        This creates a list of the form field names/values in a deterministic
+        Subclasses may want to take into account request-specific information,
-        order, pickles the result with the SECRET_KEY setting and takes an md5
-        hash of that.
-
-        Subclasses may want to take into account request-specific information
         such as the IP address.
         """
-        data = [(bf.name, bf.data or '') for bf in form] + [settings.SECRET_KEY]
+        return security_hash(request, form)
-        # Use HIGHEST_PROTOCOL because it's the most efficient. It requires
-        # Python 2.3, but Django requires 2.3 anyway, so that's OK.
-        pickled = pickle.dumps(data, pickle.HIGHEST_PROTOCOL)
-        return md5_constructor(pickled).hexdigest()
 
     def failed_hash(self, request):
         "Returns an HttpResponse in the case of an invalid security hash."

django/contrib/formtools/tests.py

@@ -4,20 +4,16 @@ from django import http
 from django.test import TestCase
 
 success_string = "Done was called!"
-test_data = {'field1': u'foo',
-             'field1_': u'asdf'}
-
 
 class TestFormPreview(preview.FormPreview):
 
     def done(self, request, cleaned_data):
         return http.HttpResponse(success_string)
 
-
 class TestForm(forms.Form):
     field1 = forms.CharField()
     field1_ = forms.CharField()
-
+    bool1 = forms.BooleanField(required=False)
 
 class PreviewTests(TestCase):
     urls = 'django.contrib.formtools.test_urls'
@@ -27,6 +23,7 @@ class PreviewTests(TestCase):
         self.preview = preview.FormPreview(TestForm)
         input_template = '<input type="hidden" name="%s" value="%s" />'
         self.input = input_template % (self.preview.unused_name('stage'), "%d")
+        self.test_data = {'field1':u'foo', 'field1_':u'asdf'}
 
     def test_unused_name(self):
         """
@@ -59,8 +56,8 @@ class PreviewTests(TestCase):
         """
         # Pass strings for form submittal and add stage variable to
         # show we previously saw first stage of the form.
-        test_data.update({'stage': 1})
+        self.test_data.update({'stage': 1})
-        response = self.client.post('/test1/', test_data)
+        response = self.client.post('/test1/', self.test_data)
         # Check to confirm stage is set to 2 in output form.
         stage = self.input % 2
         self.assertContains(response, stage, 1)
@@ -77,11 +74,30 @@ class PreviewTests(TestCase):
         """
         # Pass strings for form submittal and add stage variable to
         # show we previously saw first stage of the form.
-        test_data.update({'stage': 2})
+        self.test_data.update({'stage':2})
-        response = self.client.post('/test1/', test_data)
+        response = self.client.post('/test1/', self.test_data)
         self.failIfEqual(response.content, success_string)
-        hash = self.preview.security_hash(None, TestForm(test_data))
+        hash = self.preview.security_hash(None, TestForm(self.test_data))
-        test_data.update({'hash': hash})
+        self.test_data.update({'hash': hash})
-        response = self.client.post('/test1/', test_data)
+        response = self.client.post('/test1/', self.test_data)
+        self.assertEqual(response.content, success_string)
+
+    def test_bool_submit(self):
+        """
+        Test contrib.formtools.preview form submittal when form contains:
+        BooleanField(required=False)
+
+        Ticket: #6209 - When an unchecked BooleanField is previewed, the preview
+        form's hash would be computed with no value for ``bool1``. However, when
+        the preview form is rendered, the unchecked hidden BooleanField would be
+        rendered with the string value 'False'. So when the preview form is
+        resubmitted, the hash would be computed with the value 'False' for
+        ``bool1``. We need to make sure the hashes are the same in both cases.
+
+        """
+        self.test_data.update({'stage':2})
+        hash = self.preview.security_hash(None, TestForm(self.test_data))
+        self.test_data.update({'hash':hash, 'bool1':u'False'})
+        response = self.client.post('/test1/', self.test_data)
         self.assertEqual(response.content, success_string)
 

django/contrib/formtools/utils.py

@@ -0,0 +1,39 @@
+try:
+    import cPickle as pickle
+except ImportError:
+    import pickle
+    
+from django.conf import settings
+from django.utils.hashcompat import md5_constructor
+from django.forms import BooleanField
+
+def security_hash(request, form, *args):
+        """
+        Calculates a security hash for the given Form instance.
+
+        This creates a list of the form field names/values in a deterministic
+        order, pickles the result with the SECRET_KEY setting, then takes an md5
+        hash of that.
+        """
+        # Ensure that the hash does not change when a BooleanField's bound
+        # data is a string `False' or a boolean False.
+        # Rather than re-coding this special behaviour here, we
+        # create a dummy BooleanField and call its clean method to get a
+        # boolean True or False verdict that is consistent with
+        # BooleanField.clean()
+        dummy_bool = BooleanField(required=False)
+        def _cleaned_data(bf):
+            if isinstance(bf.field, BooleanField):
+                return dummy_bool.clean(bf.data)
+            return bf.data
+        
+        data = [(bf.name, _cleaned_data(bf) or '') for bf in form]
+        data.extend(args)
+        data.append(settings.SECRET_KEY)
+        
+        # Use HIGHEST_PROTOCOL because it's the most efficient. It requires
+        # Python 2.3, but Django requires 2.3 anyway, so that's OK.
+        pickled = pickle.dumps(data, pickle.HIGHEST_PROTOCOL)
+        
+        return md5_constructor(pickled).hexdigest()
+

django/contrib/formtools/wizard.py

@@ -12,6 +12,7 @@ from django.http import Http404
 from django.shortcuts import render_to_response
 from django.template.context import RequestContext
 from django.utils.hashcompat import md5_constructor
+from django.contrib.formtools.utils import security_hash
 
 class FormWizard(object):
     # Dictionary of extra template context variables.
@@ -140,18 +141,10 @@ class FormWizard(object):
         """
         Calculates the security hash for the given HttpRequest and Form instances.
 
-        This creates a list of the form field names/values in a deterministic
-        order, pickles the result with the SECRET_KEY setting and takes an md5
-        hash of that.
-
         Subclasses may want to take into account request-specific information,
         such as the IP address.
         """
-        data = [(bf.name, bf.data or '') for bf in form] + [settings.SECRET_KEY]
+        return security_hash(request, form)
-        # Use HIGHEST_PROTOCOL because it's the most efficient. It requires
-        # Python 2.3, but Django requires 2.3 anyway, so that's OK.
-        pickled = pickle.dumps(data, pickle.HIGHEST_PROTOCOL)
-        return md5_constructor(pickled).hexdigest()
 
     def determine_step(self, request, *args, **kwargs):
         """