#7038 closed (duplicate)
FormWizard fails on forms with BooleanField when field is unselected
Reported by: | anonymous | Owned by: | mcroydon |
---|---|---|---|
Component: | contrib.formtools | Version: | dev |
Severity: | Keywords: | FormWizard, BooleanField, hash failure | |
Cc: | rajesh.dhawan@…, dev@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description (last modified by )
The form wizard fails to match its security hash when processing a form containing at least one BooleanField where the BooleanField's checkbox is unselected (e.g. False). This results in (on my machine) the form containing the BooleanField being redisplayed. If *all* the BooleanFields on a form are selected, it appears to work correctly.
I was able to workaround (not fix) by overriding the security_hash method as follows:
def security_hash(self, request, form): for bf in form: if bf.field.__class__ != BooleanField: data = [(bf.name, bf.data or u'')] + [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, protocol=pickle.HIGHEST_PROTOCOL) return md5.new(pickled).hexdigest()
The problem appears (to my untrained eye) to be in the line assigning the value of data where bf.data = False and thus gets assigned .
I'm using SVN 7412.
Attachments (1)
Change History (17)
comment:1 by , 17 years ago
Component: | Uncategorized → django.contrib.formtools |
---|
follow-up: 4 comment:2 by , 16 years ago
comment:3 by , 16 years ago
Keywords: | hash failure added |
---|
I get this also... not an easy bug to detect when one has a wizard with many forms! maybe a better solution would be to explicitly set every unset BooleanField to False as soon as possible, that is: well before the hashing?
comment:4 by , 16 years ago
Replying to tbye@tbye.com:
When attempting to use the sample workaround above with rev. 7545 I hit an issue. Here's my updated sample to correct the problems I ran into.
def security_hash(self, request, form):
for bf in form:
if bf.field != BooleanField:
data = [(bf.name, bf.data or )] + [settings.SECRET_KEY]
This also means that only the last field per form, whatever that is, is hashed at all, as 'data' is replaced for almost every trip through the 'for'. Ergo, one might as well not have the hashing at all.
The real problem is that sql and web and boolean logic disagrees as to what a checkbox (CheckboxInput in django's case) is supposed to mean. Sql has three values for a bool: true, false, and unset. Web only has on/True and unset. Django has two types of boolean fields, using the same widget: BooleanField, which must be set and has two possible values: True and False, and NullboleanField, which is like sql in that it has three values: True, False and None (unset). The checkbox-widget however, obeys the web-rule of True vs. unset and ergo doesn't really fit with either of the fields =) See also #7051, #7190, #6937, #6914, #5957
comment:5 by , 16 years ago
Description: | modified (diff) |
---|
by , 16 years ago
Attachment: | django.contrib.formtools.7828.patch added |
---|
django.contrib.formtools.7828.patch
comment:6 by , 16 years ago
Has patch: | set |
---|
I can also reproduce this issue. The problem is with the line in wizard.py:
data = [(bf.name, bf.data or '') for bf in form] + [settings.SECRET_KEY]
Where can bf.data == False when the hash is created, and bf.data == u'False' when the hash is validated. So the hashes don't match and we get a hash error. It seems like an easy fix would be to do the following:
#data = [(bf.name, unicode(bf.data) if bf.data is not None else '') for bf in form] + [settings.SECRET_KEY]
But that fails due an odd behavior with pickling where the pickled data doesn't produce the exact binary output:
>>> import cPickle as pickle >>> >>> u'False' == unicode(False) True >>> pickle1 = pickle.dumps(u'False', pickle.HIGHEST_PROTOCOL) >>> pickle2 = pickle.dumps(unicode(False), pickle.HIGHEST_PROTOCOL) >>> pickle1 == pickle2 False >>> pickle1 '\x80\x02X\x05\x00\x00\x00Falseq\x01.' >>> pickle2 '\x80\x02X\x05\x00\x00\x00False.' >>> >>> >>> f1 = u'False' >>> f2 = unicode(False) >>> f1 == f2 True >>> pickle1 = pickle.dumps(f1, pickle.HIGHEST_PROTOCOL) >>> pickle2 = pickle.dumps(f2, pickle.HIGHEST_PROTOCOL) >>> pickle1 == pickle2 True >>> pickle1 '\x80\x02X\x05\x00\x00\x00Falseq\x01.' >>> pickle2 '\x80\x02X\x05\x00\x00\x00Falseq\x01.' >>>
It seems like a better approach would be to remove the pickle operation and use a different serialization method (e.g., sorted list of key values converted to a string) as the md5 input. I haven't done that in the attached patch and can leave that for someone else to choose to do. For now, the attached patch fixes it by expanding the list comprehension.
comment:7 by , 16 years ago
milestone: | → 1.0 |
---|
comment:8 by , 16 years ago
This problem is similar to the FormPreview
problem of #6209. I have a fix uploaded there. Whatever fix is incorporated, it should be consistent between the hash calculations of the
FormPreview
and those of the
FormWizard
.
comment:9 by , 16 years ago
Cc: | added |
---|
comment:10 by , 16 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:11 by , 16 years ago
Cc: | added |
---|
comment:12 by , 16 years ago
The patch django.contrib.formtools.7828.patch does not work for me when there are multiple blank checkboxes on a single form. It does work if there are two checkboxes where one is ticked and the other is blank, but if I leave both blank the form wizard throws me back to that step.
I am using trunk revision [8222]
After having looked at ticket #6209 I have replaced line 50 of wizard.py with the code from this patch patch-#6209.diff, so far this seems to have completely solved my problems.
I hope that this information helps.
comment:13 by , 16 years ago
Sorry the above: "I have replaced line 50 of wizard.py
"
Should read: "I have replaced line 150 of wizard.py
"
The link is correct.
comment:14 by , 16 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:15 by , 16 years ago
Resolution: | → duplicate |
---|---|
Status: | assigned → closed |
The patch to Ticket #6209 solves this using BooleanField's clean() method, which should solve your problem. Marking this as a dupe of that ticket.
When attempting to use the sample workaround above with rev. 7545 I hit an issue. Here's my updated sample to correct the problems I ran into.