Skip to content

Fix boolean validation when string is passed as value #1866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Fix boolean validation when string is passed as value #1866

wants to merge 2 commits into from

Conversation

MattBlack85
Copy link

the following example:

class BoolSerializer(serializers.Serializer):
    bool_field = serializer.BooleanField()

does behave in such case:

>>> s = BoolSerializer(data={'bool_field': 'somestring'})
>>> s.is_valid()
>>> True

@@ -454,7 +454,7 @@ def from_native(self, value):
return True
if value in ('false', 'f', 'False', '0'):
return False
return bool(value)
raise ValidationError(self.error_messages['invalid'] % _(value))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not convinced it's a good idea to raise a ValidationError within the from_native. Shouldn't it be in the validate ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect behaviour if value is True, False, 1 and 0.

@tomchristie tomchristie mentioned this pull request Sep 11, 2014
94 tasks
@tomchristie
Copy link
Member

Closing this off. On my checklist for #1800.

@MattBlack85
Copy link
Author

Any remote chance to have it in 2.x?

@tomchristie
Copy link
Member

Nope. Cracking on with 3.0, rather focus on that. The behaviour as it currently is reasonable enough in any case. If you want alternate behavior, use a custom field class.

@AeroNotix
Copy link

@tomchristie Regarding this being "reasonable" -- part of the huge amount of hate against PHP is the fact that it has automatic conversions such as this. Do you want to make DRF work like PHP?

@xordoquy
Copy link
Collaborator

@AeroNotix Please avoid such negative statements.
DRF has limited resources and putting more things into 2.x will simply delay even more the work for 3.x which will deal with this issue and make the code in this PR obsolete.

@AeroNotix
Copy link

@xordoquy I don't see the negativity. I'm genuinely surprised at the behaviour introduced in the 3.0 branch. The allowed values don't even make sense for a boolean field for the serialized types available.

    TRUE_VALUES = set(('t', 'T', 'true', 'True', 'TRUE', '1', 1, True))
    FALSE_VALUES = set(('f', 'F', 'false', 'False', 'FALSE', '0', 0, 0.0, False))

Which formats allow '0' or 0 or 0.0 to represent false? DRF supports JSON (which does not support anything other than true/false), YAML (which supports a couple of things like NO and 'N', yet you do not support them here).

Ideally the allowed values would take into account the format being parsed and validate based on those values.

@xordoquy
Copy link
Collaborator

@AeroNotix the "reasonable" part applies to the current branch.
As for 3.x I do think you'll be able to override the default fields to provide a stricter policy.

@tomchristie tomchristie mentioned this pull request Nov 5, 2014
96 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants