Opened 10 years ago

Closed 10 years ago

Last modified 4 years 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@… 10 years ago.

Download all attachments as: .zip

Change History (5)

by xiaowl@…, 10 years ago

Attachment: patch.diff added

comment:1 by Claude Paroz, 10 years ago

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 by Ciro Santilli 六四事件 法轮功 包卓轩, 8 years ago

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 8 years ago by Ciro Santilli 六四事件 法轮功 包卓轩 (previous) (diff)

comment:3 by Christian Kreuzberger, 6 years ago

A similar inconsistency still exists 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)
Version 0, edited 6 years ago by Christian Kreuzberger (next)

in reply to:  3 comment:4 by Thomas Hamann, 4 years ago

Replying to Christian Kreuzberger:

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

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.

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...

This same behaviour in combination with a Google Chrome/Chromium 'feature' leads to some very unexpected behaviour that may cause people to assume their APIs are broken.

In Chrome and many Chromium-based browsers first submitting a URL of the form:

/my/site/?show_only_assigned_tasks=true

and then re-submitting it as:

/my/site/?show_only_assigned_tasks=True

causes the browser to assume that the same URL was entered twice because the page caching mechanism apparently does not check on case-sensitivity! Apparently this is by design...

As mentioned by Christian this works out fine for BooleanFields, but due to the inconsistency in the NullBooleanField string handling the API will seem to keep failing even when using the correctly capitalized URL after first using the wrongly capitalized one in Chrome/Chromium.

There are two user-side solutions that do not involve using Christian's solution, and both are user-unfriendly:

  • empty browser cache
  • disable certain security settings

It would be better if Django 1.11 could be patched to fix the issue with the inconsistent handling of input for NullBooleanFields.

I don't know whether this same behaviour happens in Django 2.0 or above when using Chrome or Chromium, but it may be worth checking because just making a user click on intentionally wrong URLs in Chrome can result in a seemingly broken Django site. Edit: The widget is fixed in Django 2.0 and later.

Last edited 4 years ago by Thomas Hamann (previous) (diff)
Note: See TracTickets for help on using tickets.
Back to Top