Opened 5 years ago

Closed 5 years ago

Last modified 11 months ago

#22406 closed Bug (needsinfo)

Inconsistent way to parse boolean value from posted data

Reported by: xiaowl@… Owned by: nobody
Component: Forms Version: 1.6
Severity: Normal Keywords:
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

django.forms.fields.BooleanField.to_python tries to test if the given value is in ('false', '0'), but the value passed in is collected via BooleanField.widget.value_from_datadict, which uses a predefined dict {'true': True, 'false': False} to retreive boolean value, otherwise it returns bool(value).

Thus, posting a boolean field with value '0' will produce True, instead of False.


Attachments (1)

patch.diff (135 bytes) - added by xiaowl@… 5 years ago.

Download all attachments as: .zip

Change History (4)

Changed 5 years ago by xiaowl@…

Attachment: patch.diff added

comment:1 Changed 5 years ago by Claude Paroz

Resolution: needsinfo
Status: newclosed

The fact that CheckboxInput.value_from_datadict treats 0 as True is by design (see #16820).
Please provide us with a more detailed use case to show why the above behavior is problematic for you.

comment:2 Changed 2 years ago by Ciro Santilli 六四事件 法轮功 包卓轩

I am doing something like this:

  • a single AJAX view creates or deletes user up or down votes for posts. If the vote already exists, it is deleted or modified.
  • there are two types of vote differentiated by a Booleanfield (application specific, difficult to explain), let's call them typeTrue and typeFalse
  • each user can do a single vote of each type (typeTrue and typeFalse) either up or down

Now the view works as follows:

  • check if the vote of a given type already exists for a given user. This is an unique triple. This step runs: Vote.filter(booleanfield=request.POST['booleanfield'])
  • if the exact same vote exists, delete it
  • if it exists but with a different value (up or down vote), get the existing vote vote, and update it. This means a form validation: form = Vote(request.POST, instance=vote)

So now, what should my AJAX send?

If nothing, as a form submission checkbox would on False, the existing vote is not found as it should to be deleted.

If '0', the form always gives me True.

I think 'false' would work because it works for both, but it feels inconsistent. Why '0' does not work as well?

Actual code: https://github.com/cirosantilli/free-books-django-raw/blob/d4cb03d61a6f0c19ac7333ee1099e679e0acef21/free_books/views.py#L280

Last edited 2 years ago by Ciro Santilli 六四事件 法轮功 包卓轩 (previous) (diff)

comment:3 Changed 11 months ago by Christian Kreuzberger

A similar inconsistency still exists in 1.11 with BooleanFields and NullBooleanFields in django.forms:

django.forms.fields.BooleanField has the following logic:

class BooleanField(Field):
    widget = CheckboxInput

    def to_python(self, value):
        """Returns a Python boolean object."""
        # Explicitly check for the string 'False', which is what a hidden field
        # will submit for False. Also check for '0', since this is what
        # RadioSelect will provide. Because bool("True") == bool('1') == True,
        # we don't need to handle that explicitly.
        if isinstance(value, six.string_types) and value.lower() in ('false', '0'):
            value = False
        else:
            value = bool(value)
        return super(BooleanField, self).to_python(value)

So 'false', 'False',false,'0' will be evaluated to False, 'true','True',true,'1' will be evaluated to True. I guess '2' and '3' will not be converted and will be evaluated as None at this point.

django.forms.fields.NullBooleanField has a different implementation:

class NullBooleanField(BooleanField):
    """
    A field whose valid values are None, True and False. Invalid values are
    cleaned to None.
    """
    widget = NullBooleanSelect

    def to_python(self, value):
        """
        Explicitly checks for the string 'True' and 'False', which is what a
        hidden field will submit for True and False, for 'true' and 'false',
        which are likely to be returned by JavaScript serializations of forms,
        and for '1' and '0', which is what a RadioField will submit. Unlike
        the Booleanfield we need to explicitly check for True, because we are
        not using the bool() function
        """
        if value in (True, 'True', 'true', '1'):
            return True
        elif value in (False, 'False', 'false', '0'):
            return False
        else:
            return None

Essentially, the to_python method will do the same as BooleanFields to_python method, but now comes the funny part:

NullBooleanField uses the NullBooleanSelect widget, while BooleanField uses the CheckboxInput widget:

BooleanField uses django.forms.widget.CheckboxInput which evaluates 'true','True',True,'1' to True, and 'false','False',False,'0' to False - which is fine.

class CheckboxInput(Input):
    def value_from_datadict(self, data, files, name):
        if name not in data:
            # A missing value means False because HTML form submission does not
            # send results for unselected checkboxes.
            return False
        value = data.get(name)
        # Translate true and false strings to boolean values.
        values = {'true': True, 'false': False}
        if isinstance(value, six.string_types):
            value = values.get(value.lower(), value)
        return bool(value)

But NullBooleanField uses NullBooleanSelect, which as stated above, does NOT evaluate 'false' to False, and not 'true' to True. Instead, it evalutes '2' and 'True' to True, '3' and 'False' to False...

class NullBooleanSelect(Select):
    """
    A Select Widget intended to be used with NullBooleanField.
    """
    def __init__(self, attrs=None):
        choices = (
            ('1', ugettext_lazy('Unknown')),
            ('2', ugettext_lazy('Yes')),
            ('3', ugettext_lazy('No')),
        )
        super(NullBooleanSelect, self).__init__(attrs, choices)

    def format_value(self, value):
        try:
            return {True: '2', False: '3', '2': '2', '3': '3'}[value]
        except KeyError:
            return '1'

    def value_from_datadict(self, data, files, name):
        value = data.get(name)
        return {
            '2': True,
            True: True,
            'True': True,
            '3': False,
            'False': False,
            False: False,
        }.get(value)

Because of this inconsistency, we are experiencing a very funny behaviour for NullBooleanField:

django.forms.forms.BaseForm has the _clean_fields method, which calls the widgets value_from_datadict method:
value = field.widget.value_from_datadict(self.data, self.files, self.add_prefix(name))
This is done for each field, and it is happening BEFORE the to_python method of the field is called.

Essentially, this whole inconsistency leads to a funny behaviour:
If you use a BooleanField the following HTTP request will be evaluated correctly by the form:
/my/site/?show_only_assigned_tasks=true
But if you use a NullBooleanField instead of a BooleanField, the above request will not work, and the String 'true' will be evaluated as None.

TL;DR: NullBooleanSelect should be extended with a way to allow 'false' and 'true' to be evaluated:

    def value_from_datadict(self, data, files, name):
        value = data.get(name)
        return {
            '2': True,
            True: True,
            'True': True,
            'true': True,
            '3': False,
            'False': False,
            'false': False,
            False: False,
        }.get(value)
Last edited 11 months ago by Christian Kreuzberger (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top