Code

Opened 6 years ago

Closed 6 years ago

Last modified 3 years ago

#6209 closed (fixed)

FormPreview never passes security_hash validation when a BooleanField is set to False

Reported by: yimogod@… Owned by: rokclimb15
Component: contrib.formtools Version: master
Severity: Keywords: preview form
Cc: dablak@…, rbreathe@…, rajesh.dhawan@…, dev@…, bthomas@… Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: yes Patch needs improvement: yes
Easy pickings: UI/UX:

Description

when you give a boolean field in Form preview, and you give the boolean field "False" value, Django will not commit correctly.
First, if i press "preview" button, it will preview my form content correctly, but next i press "submit" button, Django give the boolean
field "True" value unexpected, and stay at this page. Next, i press "submit" button again in this page(now the boolean field has "True" value), Django can commit correctly...

this is my model

class TestPreForm(models.Model):
  test = models.CharField(max_length=20, null=True, blank=True)
  is_test = models.BooleanField(default=False)

this my views

class TestPreFormPreview(FormPreview):
  def done(self, request, cleaned_data):
    _test = cleaned_data['test']
    _is_test = cleaned_data['is_test']
    TestPreForm.objects.create(test = _test, is_test = _is_test)  
    return render_to_response('preform/form_ok.html', {'test':_test, 'is_test':_is_test})

this is my urls

(r'^preform/preview/$', TestPreFormPreview(forms.models.form_for_model(TestPreForm))),

Attachments (3)

patch-#6209.diff (4.8 KB) - added by rajeshdhawan 6 years ago.
Patch fixes issue and adds a unit test for this case
ticket-6209-r8231.diff (4.9 KB) - added by mcroydon 6 years ago.
Updated rajeshdhawan's patch to apply cleanly against r8231
patch-#6209-preview_wizard.diff (7.8 KB) - added by rajeshdhawan 6 years ago.
Revised patch to share security hash implementation between preview and wizard

Download all attachments as: .zip

Change History (21)

comment:1 Changed 6 years ago by dablak

  • Cc dablak@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 6 years ago by dablak

  • Needs tests set

comment:3 Changed 6 years ago by oyvind

Is this in latest trunk? Can you add a paste of the preview form html to dpaste? This should be handled correctly, if the hidden field has the value 'True' or 'False'

comment:4 Changed 6 years ago by guettli

  • Needs tests unset
  • Resolution set to invalid
  • Status changed from new to closed

Original poster was asked for more information. No feedback since 6 weeks. Closing it. Please reopen if the bug
is still in the latest trunk and more information is given.

comment:5 Changed 6 years ago by rbreathe@…

  • Cc rbreathe@… added
  • Resolution invalid deleted
  • Status changed from closed to reopened

I've also hit this problem.

I have a field, beta_test = BooleanField(required=False).

When I submit the form with the checkbox unticked, the preview renders as I expect: the template snippet "{% if form.beta_test.data %}True{% else %}False{% endif %}, {{ form.beta_test.data }}" renders as "False, False".

However, when I confirm the submission, the preview page is re-rendered, the previous snippet rendering "True, False"; the contents of the <form>, with the exception of the hash field, are identical but for the ordering of attributes within the various <input> fields.

If the field is ticked for the original preview, the submission works as expected first time (the preview snippet producing "True, on").

comment:6 Changed 6 years ago by Robin Breathe <rbreathe@…>

  • milestone set to 1.0 alpha

comment:7 Changed 6 years ago by ElliottM

  • Triage Stage changed from Unreviewed to Accepted

comment:8 Changed 6 years ago by ElliottM

  • Summary changed from when you give a boolean field in Form preview, and you give the boolean field "False" value, Django will not commit rightly to FormPreview never passes security_hash validation when a BooleanField is set to False

comment:9 Changed 6 years ago by ElliottM

  • milestone changed from 1.0 alpha to 1.0

comment:10 Changed 6 years ago by rajeshdhawan

  • Cc rajesh.dhawan@… added
  • Owner changed from nobody to rajeshdhawan
  • Status changed from reopened to new

I'll take a crack at this.

Changed 6 years ago by rajeshdhawan

Patch fixes issue and adds a unit test for this case

comment:11 Changed 6 years ago by rajeshdhawan

  • Has patch set
  • Needs tests set

comment:12 Changed 6 years ago by mcroydon

  • Owner changed from rajeshdhawan to mcroydon
  • Status changed from new to assigned

Changed 6 years ago by mcroydon

Updated rajeshdhawan's patch to apply cleanly against r8231

comment:13 Changed 6 years ago by mcroydon

  • Owner changed from mcroydon to nobody
  • Status changed from assigned to new

comment:14 Changed 6 years ago by adamjforster

  • Cc dev@… added

comment:15 Changed 6 years ago by bthomas

  • Cc bthomas@… added
  • Patch needs improvement set

Ticket #7038 was marked as a duplicate of this, but will not be fixed unless security_hash in wizard.py is patched as well. Maybe the fixed version of security_hash should be moved elsewhere so both FormPreview and FormWizard can share it. Either the patch for this ticket needs to fix FormWizard as well, or #7038 needs to be reopened.

Changed 6 years ago by rajeshdhawan

Revised patch to share security hash implementation between preview and wizard

comment:16 Changed 6 years ago by rokclimb15

  • Owner changed from nobody to rokclimb15
  • Status changed from new to assigned

Latest patch works very nicely for me. Thanks everyone.

comment:17 Changed 6 years ago by jacob

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [8597]) Fixed #6209: handle BooleanFields in FormPreview and FormWizard. In the process, broke the the security hash calculation out to a helper function. Thanks to mcroydon and rajeshdhawan.

comment:18 Changed 3 years ago by jacob

  • milestone 1.0 deleted

Milestone 1.0 deleted

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.