#22406 closed Bug (needsinfo)
Inconsistent way to parse boolean value from posted data
Reported by: | 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)
Change History (5)
by , 11 years ago
Attachment: | patch.diff added |
---|
comment:1 by , 11 years ago
Resolution: | → needsinfo |
---|---|
Status: | new → closed |
comment:2 by , 9 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:
Vote.filter(booleanfield=request.POST['booleanfield'])
- if yes, delete it
- else, get the existing vote
vote
, and create or 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?
follow-up: 4 comment:3 by , 7 years ago
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)
comment:4 by , 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.
The fact that
CheckboxInput.value_from_datadict
treats0
asTrue
is by design (see #16820).Please provide us with a more detailed use case to show why the above behavior is problematic for you.