From 6056ab1beed04f53013ee83fc82303b5c42c9fbd Mon Sep 17 00:00:00 2001 From: Jacob Kaplan-Moss Date: Tue, 26 Aug 2008 20:19:12 +0000 Subject: [PATCH] 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 | 15 +++-------- django/contrib/formtools/tests.py | 40 ++++++++++++++++++++--------- django/contrib/formtools/utils.py | 39 ++++++++++++++++++++++++++++ django/contrib/formtools/wizard.py | 11 ++------ 4 files changed, 73 insertions(+), 32 deletions(-) create mode 100644 django/contrib/formtools/utils.py diff --git a/django/contrib/formtools/preview.py b/django/contrib/formtools/preview.py index c56fdff628..856343a0df 100644 --- a/django/contrib/formtools/preview.py +++ b/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 - 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 + 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] - # 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() + return security_hash(request, form) def failed_hash(self, request): "Returns an HttpResponse in the case of an invalid security hash." diff --git a/django/contrib/formtools/tests.py b/django/contrib/formtools/tests.py index 2ea0cc90a4..b1e2cb7c76 100644 --- a/django/contrib/formtools/tests.py +++ b/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 = '' 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}) - response = self.client.post('/test1/', test_data) + self.test_data.update({'stage': 1}) + 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}) - response = self.client.post('/test1/', test_data) + self.test_data.update({'stage':2}) + response = self.client.post('/test1/', self.test_data) self.failIfEqual(response.content, success_string) - hash = self.preview.security_hash(None, TestForm(test_data)) - test_data.update({'hash': hash}) - response = self.client.post('/test1/', test_data) + hash = self.preview.security_hash(None, TestForm(self.test_data)) + self.test_data.update({'hash': hash}) + 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) diff --git a/django/contrib/formtools/utils.py b/django/contrib/formtools/utils.py new file mode 100644 index 0000000000..90b4fe650b --- /dev/null +++ b/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() + diff --git a/django/contrib/formtools/wizard.py b/django/contrib/formtools/wizard.py index 984b6e487e..f764ddd0da 100644 --- a/django/contrib/formtools/wizard.py +++ b/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] - # 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() + return security_hash(request, form) def determine_step(self, request, *args, **kwargs): """